Fix two CloneNonCycle issues

CloneNonCycle() tries to detect cyclic object references without copying
them. There are two issues:
-- for elements in an array or a dictionary, they should be able to
refer to the same object, which are not cyclic;
-- for cyclic referenced elements in an array or a dictionary, do not
clone the element at all. Having nullptr or <key, nullptr> as an element,
like we did before, might cause crash when the element being accessed.

BUG=chromium:701860

Change-Id: Id0304accde76ed06fa5ce640994c7628359600fb
Reviewed-on: https://pdfium-review.googlesource.com/3156
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

1 files changed, 3 insertions, 2 deletions

diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index 653ef45067..d4e4080d31 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -68,8 +68,9 @@ std::unique_ptr<CPDF_Object> CPDF_Dictionary::CloneNonCyclic(
   auto pCopy = pdfium::MakeUnique<CPDF_Dictionary>(m_pPool);
   for (const auto& it : *this) {
     if (!pdfium::ContainsKey(*pVisited, it.second.get())) {
-      pCopy->m_Map.insert(std::make_pair(
-          it.first, it.second->CloneNonCyclic(bDirect, pVisited)));
+      std::set<const CPDF_Object*> visited(*pVisited);
+      if (auto obj = it.second->CloneNonCyclic(bDirect, &visited))
+        pCopy->m_Map.insert(std::make_pair(it.first, std::move(obj)));
     }
   }
   return std::move(pCopy);