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>
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());
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;
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);
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(); }
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)); } }
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)