commit 09ed075f13eaba42132cc00ac2cfbec3c281a48c
author Lei Zhang <thestig@chromium.org> Tue Jul 13 16:57:50 2021 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jul 13 16:57:50 2021 +0000
tree 9043f5d03ce827a7856b73bc81a25c42e99cbb52
parent 51fc745ccf072ce1d2a75ee54509e259aafabbc3

Avoid passing invalid keys to CPDF_Dictionary::SetFor().

Consolidate the logic for checking key validity into
CPDF_Dictionary::IsValidKey(), then use IsValidKey() in several places
that set dictionary keys.

Bug: chromium:1226299,chromium:1226364,chromium:1226365
Change-Id: I63d40edb11094a12fcf32ad51d9c62e5f272bd10
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/83050
Reviewed-by: Hui Yingst <nigi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fpdfapi/page/cpdf_streamcontentparser.cpp

diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 6e1e3b2..75dc551 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -618,7 +618,7 @@
     auto word = m_pSyntax->GetWord();
     ByteString key(word.Last(word.GetLength() - 1));
     auto pObj = m_pSyntax->ReadNextObject(false, false, 0);
-    if (!key.IsEmpty()) {
+    if (CPDF_Dictionary::IsValidKey(key)) {
       if (pObj && !pObj->IsInline()) {
         pDict->SetNewFor<CPDF_Reference>(key, m_pDocument.Get(),
                                          pObj->GetObjNum());

core/fpdfapi/page/cpdf_streamparser.cpp

diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp
index da16168..4b5e1ca 100644
--- a/core/fpdfapi/page/cpdf_streamparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamparser.cpp
@@ -334,7 +334,7 @@
       if (!pObj)
         return nullptr;
 
-      if (!key.IsEmpty())
+      if (CPDF_Dictionary::IsValidKey(key))
         pDict->SetFor(key, std::move(pObj));
     }
     return pDict;

core/fpdfapi/parser/cpdf_dictionary.cpp

diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index daeb765..0494372 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -82,6 +82,11 @@
   return pCopy;
 }
 
+// static
+bool CPDF_Dictionary::IsValidKey(const ByteString& key) {
+  return !key.IsEmpty() && key.AsStringView().IsASCII();
+}
+
 const CPDF_Object* CPDF_Dictionary::GetObjectFor(const ByteString& key) const {
   auto it = m_Map.find(key);
   return it != m_Map.end() ? it->second.Get() : nullptr;
@@ -210,8 +215,7 @@
 
 CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key,
                                      RetainPtr<CPDF_Object> pObj) {
-  CHECK(!key.IsEmpty());
-  CHECK(key.AsStringView().IsASCII());
+  CHECK(IsValidKey(key));
   CHECK(!IsLocked());
   if (!pObj) {
     m_Map.erase(key);

core/fpdfapi/parser/cpdf_dictionary.h

diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index 715112b..ef545e8 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -41,6 +41,9 @@
   bool WriteTo(IFX_ArchiveStream* archive,
                const CPDF_Encryptor* encryptor) const override;
 
+  // `key` must be non-empty and ASCII, per PDF 32000 standard, section 7.2.1.
+  static bool IsValidKey(const ByteString& key);
+
   bool IsLocked() const { return !!m_LockCount; }
 
   size_t size() const { return m_Map.size(); }

core/fpdfapi/parser/cpdf_syntax_parser.cpp

diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
index 2560e98..6d837a3 100644
--- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
@@ -592,8 +592,7 @@
       // `key` has to be "/X" at the minimum.
       if (key.GetLength() > 1) {
         ByteString key_no_slash(key.raw_str() + 1, key.GetLength() - 1);
-        // `key_no_slash` must be ASCII, per PDF 32000 standard, section 7.2.1.
-        if (key_no_slash.AsStringView().IsASCII())
+        if (CPDF_Dictionary::IsValidKey(key_no_slash))
           pDict->SetFor(key_no_slash, std::move(pObj));
       }
     }

core/fpdfdoc/cpdf_generateap.cpp

diff --git a/core/fpdfdoc/cpdf_generateap.cpp b/core/fpdfdoc/cpdf_generateap.cpp
index 488d504..29efe16 100644
--- a/core/fpdfdoc/cpdf_generateap.cpp
+++ b/core/fpdfdoc/cpdf_generateap.cpp
@@ -943,6 +943,9 @@
     return;
 
   ByteString font_name = font.value();
+  if (!CPDF_Dictionary::IsValidKey(font_name))
+    return;
+
   CFX_Color crText = fpdfdoc::CFXColorFromString(DA);
   CPDF_Dictionary* pDRDict = pFormDict->GetDictFor("DR");
   if (!pDRDict)