commit 6212b78169a615ed78ffc9c0cbe193de6d4b7849
author Lei Zhang <thestig@chromium.org> Mon Nov 08 20:01:21 2021 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Nov 08 20:01:21 2021 +0000
tree e30350f0b8be1e1efc3bff7c4188e46230e71681
parent 2d843bfa967c5fdd0c47799e34355bf67e7c5022

Trim excess /Limits array entries in name trees.

Add a new CPDF_NameTree test case for /Limits arrays with excess
elements. Check for this condition inside cpdf_nametree.cpp, where the
GetNodeLimitsMaybeSwap() helper function becomes
GetNodeLimitsAndSanitize().

Change-Id: Iaa06eeb65381cfad646560e0409c43b4d88765dc
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/86910
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

core/fpdfdoc/cpdf_nametree.cpp

diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp
index 546a573..c8cf0b4 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -22,7 +22,8 @@
 
 constexpr int kNameTreeMaxRecursion = 32;
 
-std::pair<WideString, WideString> GetNodeLimitsMaybeSwap(CPDF_Array* pLimits) {
+std::pair<WideString, WideString> GetNodeLimitsAndSanitize(
+    CPDF_Array* pLimits) {
   DCHECK(pLimits);
   WideString csLeft = pLimits->GetUnicodeTextAt(0);
   WideString csRight = pLimits->GetUnicodeTextAt(1);
@@ -33,6 +34,8 @@
     csLeft = pLimits->GetUnicodeTextAt(0);
     csRight = pLimits->GetUnicodeTextAt(1);
   }
+  while (pLimits->size() > 2)
+    pLimits->RemoveAt(pLimits->size() - 1);
   return {csLeft, csRight};
 }
 
@@ -82,7 +85,7 @@
   WideString csLeft;
   WideString csRight;
   if (pLimits)
-    std::tie(csLeft, csRight) = GetNodeLimitsMaybeSwap(pLimits);
+    std::tie(csLeft, csRight) = GetNodeLimitsAndSanitize(pLimits);
 
   CPDF_Array* pNames = pNode->GetArrayFor("Names");
   if (pNames) {
@@ -170,7 +173,7 @@
   if (pLimits) {
     WideString csLeft;
     WideString csRight;
-    std::tie(csLeft, csRight) = GetNodeLimitsMaybeSwap(pLimits);
+    std::tie(csLeft, csRight) = GetNodeLimitsAndSanitize(pLimits);
     // Skip this node if the name to look for is smaller than its lower limit.
     if (csName.Compare(csLeft) < 0)
       return nullptr;

core/fpdfdoc/cpdf_nametree_unittest.cpp

diff --git a/core/fpdfdoc/cpdf_nametree_unittest.cpp b/core/fpdfdoc/cpdf_nametree_unittest.cpp
index eb41d97..36617e7 100644
--- a/core/fpdfdoc/cpdf_nametree_unittest.cpp
+++ b/core/fpdfdoc/cpdf_nametree_unittest.cpp
@@ -39,6 +39,7 @@
   ASSERT_TRUE(node);
   const CPDF_Array* limits = node->GetArrayFor("Limits");
   ASSERT_TRUE(limits);
+  EXPECT_EQ(2u, limits->size());
   const CPDF_String* left = ToString(limits->GetObjectAt(0));
   ASSERT_TRUE(left);
   const CPDF_String* right = ToString(limits->GetObjectAt(1));
@@ -124,6 +125,28 @@
   EXPECT_EQ(100, pNumber->GetInteger());
 }
 
+TEST(cpdf_nametree, GetFromTreeWithLimitsArrayWith4Items) {
+  // After creating a name tree, mutate a /Limits array so it has excess
+  // elements.
+  auto pRootDict = pdfium::MakeRetain<CPDF_Dictionary>();
+  FillNameTreeDict(pRootDict.Get());
+  CPDF_Dictionary* pKid1 = pRootDict->GetArrayFor("Kids")->GetDictAt(0);
+  CPDF_Dictionary* pGrandKid3 = pKid1->GetArrayFor("Kids")->GetDictAt(1);
+  CPDF_Array* pLimits = pGrandKid3->GetArrayFor("Limits");
+  ASSERT_EQ(2u, pLimits->size());
+  pLimits->AppendNew<CPDF_Number>(5);
+  pLimits->AppendNew<CPDF_Number>(6);
+  ASSERT_EQ(4u, pLimits->size());
+  std::unique_ptr<CPDF_NameTree> name_tree =
+      CPDF_NameTree::CreateForTesting(pRootDict.Get());
+
+  const CPDF_Number* pNumber = ToNumber(name_tree->LookupValue(L"9.txt"));
+  ASSERT_TRUE(pNumber);
+  EXPECT_EQ(999, pNumber->GetInteger());
+  CheckLimitsArray(pKid1, "1.txt", "9.txt");
+  CheckLimitsArray(pGrandKid3, "9.txt", "9.txt");
+}
+
 TEST(cpdf_nametree, AddIntoNames) {
   // Set up a name tree with a single Names array.
   auto pRootDict = pdfium::MakeRetain<CPDF_Dictionary>();