summaryrefslogtreecommitdiff
path: root/core/fpdfapi/parser
diff options
context:
space:
mode:
authorWei Li <weili@chromium.org>2017-03-23 09:45:04 -0700
committerChromium commit bot <commit-bot@chromium.org>2017-03-23 17:05:05 +0000
commit6bdd824188bc9a2e6b24b5752a3170ce10185c1d (patch)
tree272c426e10e66315ef2993d2d8712029aa6a90b5 /core/fpdfapi/parser
parent409b663d532d4d6f09a1188fa3b9ac4044708bc4 (diff)
downloadpdfium-6bdd824188bc9a2e6b24b5752a3170ce10185c1d.tar.xz
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>
Diffstat (limited to 'core/fpdfapi/parser')
-rw-r--r--core/fpdfapi/parser/cpdf_array.cpp7
-rw-r--r--core/fpdfapi/parser/cpdf_dictionary.cpp5
-rw-r--r--core/fpdfapi/parser/cpdf_object_unittest.cpp6
3 files changed, 11 insertions, 7 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);
}
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);
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index 9285993c52..b25d40a029 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -761,7 +761,7 @@ TEST(PDFArrayTest, CloneDirectObject) {
std::unique_ptr<CPDF_Array> cloned_array =
ToArray(std::move(cloned_array_object));
- ASSERT_EQ(1U, cloned_array->GetCount());
+ ASSERT_EQ(0U, cloned_array->GetCount());
CPDF_Object* cloned_obj = cloned_array->GetObjectAt(0);
EXPECT_FALSE(cloned_obj);
}
@@ -795,7 +795,7 @@ TEST(PDFDictionaryTest, CloneDirectObject) {
std::unique_ptr<CPDF_Dictionary> cloned_dict =
ToDictionary(std::move(cloned_dict_object));
- ASSERT_EQ(1U, cloned_dict->GetCount());
+ ASSERT_EQ(0U, cloned_dict->GetCount());
CPDF_Object* cloned_obj = cloned_dict->GetObjectFor("foo");
EXPECT_FALSE(cloned_obj);
}
@@ -859,7 +859,7 @@ TEST(PDFObjectTest, CloneCheckLoop) {
CPDF_Object* cloned_arr = cloned_dict->GetObjectFor("arr");
ASSERT_TRUE(cloned_arr);
ASSERT_TRUE(cloned_arr->IsArray());
- EXPECT_EQ(1u, cloned_arr->AsArray()->GetCount());
+ EXPECT_EQ(0U, cloned_arr->AsArray()->GetCount());
// Recursively referenced object is not cloned.
EXPECT_EQ(nullptr, cloned_arr->AsArray()->GetObjectAt(0));
}