Tolerate bad /Size in V5 xrefs
When the /Size in a V5 xref is wrong, CPDF_Parser currently rejects xref
sections that have object numbers that goes over the incorrect size
limit. Other PDF implementations do not do this, and PDFium does not do
this for V4 xrefs. Change the code in CPDF_Parser::LoadCrossRefV5() that
is too strict and ignore the /Size.
Bug: chromium:1288804
Change-Id: I74d3f4ed548b71923c6e8b0c3948ff5defae3e8e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/108832
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Nigi <nigi@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index d571ef6..add1f23 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -810,17 +810,33 @@
pdfium::span<const uint8_t> seg_span = data_span.subspan(
segindex * total_width, index.obj_count * total_width);
- FX_SAFE_UINT32 dwMaxObjNum = index.start_obj_num;
- dwMaxObjNum += index.obj_count;
- uint32_t dwV5Size =
- m_CrossRefTable->objects_info().empty() ? 0 : GetLastObjNum() + 1;
- if (!dwMaxObjNum.IsValid() || dwMaxObjNum.ValueOrDie() > dwV5Size)
+ FX_SAFE_UINT32 safe_new_size = index.start_obj_num;
+ safe_new_size += index.obj_count;
+ if (!safe_new_size.IsValid()) {
continue;
+ }
+
+ // Until SetObjectMapSize() below has been called by a prior loop iteration,
+ // `current_size` is based on the /Size value parsed in LoadCrossRefV5().
+ // PDFs may not always have the correct /Size. In this case, other PDF
+ // implementations ignore the incorrect size, and PDFium also ignores
+ // incorrect size in trailers for V4 xrefs.
+ const uint32_t current_size =
+ m_CrossRefTable->objects_info().empty() ? 0 : GetLastObjNum() + 1;
+ // So allow `new_size` to be greater than `current_size`, but avoid going
+ // over `kMaxXRefSize`. This works just fine because the loop below checks
+ // against `kMaxObjectNumber`, and the two "max" constants are in sync.
+ const uint32_t new_size =
+ std::min<uint32_t>(safe_new_size.ValueOrDie(), kMaxXRefSize);
+ if (new_size > current_size) {
+ m_CrossRefTable->SetObjectMapSize(new_size);
+ }
for (uint32_t i = 0; i < index.obj_count; ++i) {
const uint32_t obj_num = index.start_obj_num + i;
- if (obj_num >= CPDF_Parser::kMaxObjectNumber)
+ if (obj_num >= kMaxObjectNumber) {
break;
+ }
ProcessCrossRefV5Entry(seg_span.subspan(i * total_width, total_width),
field_widths, obj_num);
diff --git a/core/fpdfapi/parser/cpdf_parser_unittest.cpp b/core/fpdfapi/parser/cpdf_parser_unittest.cpp
index eaf673f..0759807 100644
--- a/core/fpdfapi/parser/cpdf_parser_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_parser_unittest.cpp
@@ -809,12 +809,14 @@
ASSERT_TRUE(parser().GetCrossRefTable());
const auto& objects_info = parser().GetCrossRefTable()->objects_info();
- // TODO(crbug.com/1288804): This expectation is wrong. Other PDF parsers
- // tolerate the incorrect /Size.
- CPDF_Parser::ObjectInfo expected_result[2];
+ CPDF_Parser::ObjectInfo expected_result[3];
expected_result[0].type = CPDF_Parser::ObjectType::kNormal;
expected_result[0].pos = 0;
- expected_result[1].type = CPDF_Parser::ObjectType::kFree;
+ expected_result[1].type = CPDF_Parser::ObjectType::kNormal;
+ expected_result[1].pos = 15;
+ expected_result[2].type = CPDF_Parser::ObjectType::kNormal;
+ expected_result[2].pos = 18;
EXPECT_THAT(objects_info, ElementsAre(Pair(2, expected_result[0]),
- Pair(80, expected_result[1])));
+ Pair(80, expected_result[1]),
+ Pair(81, expected_result[2])));
}
