commit 329a2d285470276c85267856543d9a13e1bc46af
author Tom Sepez <tsepez@chromium.org> Thu Sep 22 18:10:01 2022 +0000
committer Tom Sepez <tsepez@chromium.org> Thu Sep 22 18:10:01 2022 +0000
tree 91690dc1581f03598d7c36bf774a346a83c9d082
parent 873bffafa8fd75541250149685153d7c083ee950

[M102-LTS] Enforce maximum legal object number during linearized parses.

- Watch for overflow of object numbers.
- Re-validate CPDF_Object pointer after notification in CPDF_FormField.

Bug: chromium:1358090
Change-Id: I1effd8f47277d177c804dd14b20b101e71780067
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97130
Commit-Queue: Tom Sepez <tsepez@chromium.org>
(cherry picked from commit 81ab3354f79765438bad0e9d683adcfce96727fa)
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97751
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Artem Sumaneev <asumaneev@google.com>

core/fpdfapi/parser/cpdf_hint_tables.cpp

diff --git a/core/fpdfapi/parser/cpdf_hint_tables.cpp b/core/fpdfapi/parser/cpdf_hint_tables.cpp
index 3445e90..0f2632c 100644
--- a/core/fpdfapi/parser/cpdf_hint_tables.cpp
+++ b/core/fpdfapi/parser/cpdf_hint_tables.cpp
@@ -13,6 +13,7 @@
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_document.h"
 #include "core/fpdfapi/parser/cpdf_linearized_header.h"
+#include "core/fpdfapi/parser/cpdf_parser.h"
 #include "core/fpdfapi/parser/cpdf_read_validator.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/cpdf_stream_acc.h"
@@ -101,7 +102,7 @@
 
   // Item 1: The least number of objects in a page.
   const uint32_t dwObjLeastNum = hStream->GetBits(32);
-  if (!dwObjLeastNum)
+  if (!dwObjLeastNum || dwObjLeastNum >= CPDF_Parser::kMaxObjectNumber)
     return false;
 
   // Item 2: The location of the first page's page object.
@@ -164,7 +165,7 @@
   m_PageInfos[nFirstPageNum].set_start_obj_num(
       m_pLinearized->GetFirstPageObjNum());
   // The object number of remaining pages starts from 1.
-  uint32_t dwStartObjNum = 1;
+  FX_SAFE_UINT32 dwStartObjNum = 1;
   for (uint32_t i = 0; i < nPages; ++i) {
     FX_SAFE_UINT32 safeDeltaObj = hStream->GetBits(dwDeltaObjectsBits);
     safeDeltaObj += dwObjLeastNum;
@@ -173,8 +174,12 @@
     m_PageInfos[i].set_objects_count(safeDeltaObj.ValueOrDie());
     if (i == nFirstPageNum)
       continue;
-    m_PageInfos[i].set_start_obj_num(dwStartObjNum);
+    m_PageInfos[i].set_start_obj_num(dwStartObjNum.ValueOrDie());
     dwStartObjNum += m_PageInfos[i].objects_count();
+    if (!dwStartObjNum.IsValid() ||
+        dwStartObjNum.ValueOrDie() >= CPDF_Parser::kMaxObjectNumber) {
+      return false;
+    }
   }
   hStream->ByteAlign();
 

core/fpdfdoc/cpdf_formfield.cpp

diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index e882098..c27576e 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -178,14 +178,15 @@
     case kRichText:
     case kFile:
     default: {
-      const CPDF_Object* pDV = GetDefaultValueObject();
       WideString csDValue;
-      if (pDV)
-        csDValue = pDV->GetUnicodeText();
-
       WideString csValue;
       {
-        // Limit the scope of |pV| because it may get invalidated below.
+        // Limit scope of |pDV| and |pV| because they may get invalidated
+        // during notification below.
+        const CPDF_Object* pDV = GetDefaultValueObject();
+        if (pDV)
+          csDValue = pDV->GetUnicodeText();
+
         const CPDF_Object* pV = GetValueObject();
         if (pV)
           csValue = pV->GetUnicodeText();
@@ -195,21 +196,26 @@
       if (!bHasRV && (csDValue == csValue))
         return false;
 
-      if (!NotifyBeforeValueChange(csDValue)) {
+      if (!NotifyBeforeValueChange(csDValue))
         return false;
-      }
-      if (pDV) {
-        RetainPtr<CPDF_Object> pClone = pDV->Clone();
-        if (!pClone)
-          return false;
 
-        m_pDict->SetFor(pdfium::form_fields::kV, std::move(pClone));
-        if (bHasRV) {
-          m_pDict->SetFor("RV", pDV->Clone());
+      {
+        // Limit scope of |pDV| because it may get invalidated during
+        // notification below.
+        const CPDF_Object* pDV = GetDefaultValueObject();
+        if (pDV) {
+          RetainPtr<CPDF_Object> pClone = pDV->Clone();
+          if (!pClone)
+            return false;
+
+          m_pDict->SetFor(pdfium::form_fields::kV, std::move(pClone));
+          if (bHasRV) {
+            m_pDict->SetFor("RV", pDV->Clone());
+          }
+        } else {
+          m_pDict->RemoveFor(pdfium::form_fields::kV);
+          m_pDict->RemoveFor("RV");
         }
-      } else {
-        m_pDict->RemoveFor(pdfium::form_fields::kV);
-        m_pDict->RemoveFor("RV");
       }
       NotifyAfterValueChange();
       break;