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, 5 insertions, 2 deletions

diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp
index bf8b51e604..f3c23f37be 100644
--- a/core/fpdfapi/parser/cpdf_array.cpp
+++ b/core/fpdfapi/parser/cpdf_array.cpp
@@ -57,8 +57,11 @@ std::unique_ptr<CPDF_Object> CPDF_Array::CloneNonCyclic(
   pVisited->insert(this);
   auto pCopy = pdfium::MakeUnique<CPDF_Array>();
   for (const auto& pValue : m_Objects) {
-    if (!pdfium::ContainsKey(*pVisited, pValue.get()))
-      pCopy->m_Objects.push_back(pValue->CloneNonCyclic(bDirect, pVisited));
+    if (!pdfium::ContainsKey(*pVisited, pValue.get())) {
+      std::set<const CPDF_Object*> visited(*pVisited);
+      if (auto obj = pValue->CloneNonCyclic(bDirect, &visited))
+        pCopy->m_Objects.push_back(std::move(obj));
+    }
   }
   return std::move(pCopy);
 }