Limit xref size in CPDF_Parser::LoadCrossRefStream()
Check the size against `kMaxXRefSize` before calling
CPDF_CrossRefTable::SetObjectMapSize(), like the other caller for
SetObjectMapSize().
Bug: 439237853
Change-Id: Ib1bae822fc84c1c503d722669c922d36a6076b6b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/135470
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index ec6d99f..782e88c 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -844,13 +844,19 @@
}
RetainPtr<const CPDF_Dictionary> dict = pStream->GetDict();
- int32_t prev = dict->GetIntegerFor("Prev");
+ const int32_t prev = dict->GetIntegerFor("Prev");
if (prev < 0) {
return false;
}
- int32_t size = dict->GetIntegerFor("Size");
- if (size < 0) {
+ // If /Size is negative or way too big, then it is obvious wrong.
+ // Immediately reject these cases, so `cross_ref_table_` does not get into a
+ // bad state.
+ //
+ // For a /Size of 0 or other issues, just ignore them. See comments in the
+ // for-loop below.
+ const int32_t size = dict->GetIntegerFor("Size");
+ if (size < 0 || size >= kMaxXRefSize) {
return false;
}
diff --git a/core/fpdfapi/parser/cpdf_parser_unittest.cpp b/core/fpdfapi/parser/cpdf_parser_unittest.cpp
index ea46152..f212c69 100644
--- a/core/fpdfapi/parser/cpdf_parser_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_parser_unittest.cpp
@@ -481,25 +481,7 @@
"14\n"
"%%EOF\n";
ASSERT_TRUE(parser().InitTestFromBuffer(kData));
- EXPECT_EQ(CPDF_Parser::SUCCESS, parser().StartParseInternal());
- EXPECT_FALSE(parser().xref_table_rebuilt());
- ASSERT_TRUE(parser().GetCrossRefTableForTesting());
- const auto& objects_info =
- parser().GetCrossRefTableForTesting()->objects_info();
-
- // This should be the only object from table. Subsequent objects have object
- // numbers that are too big.
- const CPDF_CrossRefTable::ObjectInfo only_valid_object = {
- .type = CPDF_CrossRefTable::ObjectType::kNormal, .pos = 0};
-
- // TODO(thestig): Should the xref table contain object 4194304?
- // Consider reworking CPDF_Parser's object representation to avoid having to
- // store this placeholder object.
- const CPDF_CrossRefTable::ObjectInfo placeholder_object = {
- .type = CPDF_CrossRefTable::ObjectType::kFree, .pos = 0};
-
- EXPECT_THAT(objects_info, ElementsAre(Pair(4194303, only_valid_object),
- Pair(4194304, placeholder_object)));
+ EXPECT_EQ(CPDF_Parser::FORMAT_ERROR, parser().StartParseInternal());
}
TEST_F(ParserXRefTest, XrefObjectIndicesTooBig2) {
@@ -526,25 +508,7 @@
"14\n"
"%%EOF\n";
ASSERT_TRUE(parser().InitTestFromBuffer(kData));
- EXPECT_EQ(CPDF_Parser::SUCCESS, parser().StartParseInternal());
- EXPECT_FALSE(parser().xref_table_rebuilt());
- ASSERT_TRUE(parser().GetCrossRefTableForTesting());
- const auto& objects_info =
- parser().GetCrossRefTableForTesting()->objects_info();
-
- // This should be the only object from table. Subsequent objects have object
- // numbers that are too big.
- const CPDF_CrossRefTable::ObjectInfo only_valid_object = {
- .type = CPDF_CrossRefTable::ObjectType::kNormal, .pos = 0};
-
- // TODO(thestig): Should the xref table contain object 4194305?
- // Consider reworking CPDF_Parser's object representation to avoid having to
- // store this placeholder object.
- const CPDF_CrossRefTable::ObjectInfo placeholder_object = {
- .type = CPDF_CrossRefTable::ObjectType::kFree, .pos = 0};
-
- EXPECT_THAT(objects_info, ElementsAre(Pair(4194303, only_valid_object),
- Pair(4194305, placeholder_object)));
+ EXPECT_EQ(CPDF_Parser::FORMAT_ERROR, parser().StartParseInternal());
}
TEST_F(ParserXRefTest, XrefHasInvalidArchiveObjectNumber) {
