Prevent dangling pointer in CPDF_StructTree::AddPageNode()
CPDF_StructTree should only call SetParent() for an element if calling
UpdateKidIfElement() on the parent for the element succeeds. Add an
embedder test to demonstrate this, using a deliberately bad copy of
tagged_table.pdf.
Bug: chromium:1443100
Change-Id: Id0164943307258b47afc26b1f358c119e47f7a07
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/107592
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfdoc/cpdf_structtree.cpp b/core/fpdfdoc/cpdf_structtree.cpp
index a8eb0a7..2b00117 100644
--- a/core/fpdfdoc/cpdf_structtree.cpp
+++ b/core/fpdfdoc/cpdf_structtree.cpp
@@ -123,11 +123,12 @@
if (!pParentElement)
return pElement;
- if (!pParentElement->UpdateKidIfElement(pDict, pElement.Get()))
+ if (!pParentElement->UpdateKidIfElement(pDict, pElement.Get())) {
map->erase(key);
+ return pElement;
+ }
pElement->SetParent(pParentElement.Get());
-
return pElement;
}
diff --git a/fpdfsdk/fpdf_structtree_embeddertest.cpp b/fpdfsdk/fpdf_structtree_embeddertest.cpp
index 472a3a9..0b9ca77 100644
--- a/fpdfsdk/fpdf_structtree_embeddertest.cpp
+++ b/fpdfsdk/fpdf_structtree_embeddertest.cpp
@@ -831,3 +831,18 @@
UnloadPage(page);
}
+
+TEST_F(FPDFStructTreeEmbedderTest, Bug1443100) {
+ ASSERT_TRUE(OpenDocument("tagged_table_bad_parent.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ {
+ // Calling these APIs should not trigger a dangling pointer.
+ ScopedFPDFStructTree struct_tree(FPDF_StructTree_GetForPage(page));
+ ASSERT_TRUE(struct_tree);
+ ASSERT_EQ(1, FPDF_StructTree_CountChildren(struct_tree.get()));
+ }
+
+ UnloadPage(page);
+}
diff --git a/testing/resources/tagged_table_bad_parent.in b/testing/resources/tagged_table_bad_parent.in
new file mode 100644
index 0000000..8175cba
--- /dev/null
+++ b/testing/resources/tagged_table_bad_parent.in
@@ -0,0 +1,217 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /StructTreeRoot 8 0 R
+ /Lang (en-US)
+ /MarkInfo <<
+ /Marked true
+ >>
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Contents 4 0 R
+ /MediaBox [0 0 612 792]
+ /Group <<
+ /CS /DeviceRGB
+ /I true
+ /S /Transparency
+ >>
+ /Resources <<
+ /ProcSet [/PDF /ImageC /ImageI /ImageB]
+ /XObject <<
+ /Tr8 5 0 R
+ /Im7 6 0 R
+ >>
+ /ExtGState <<
+ /EGS9 7 0 R
+ >>
+ >>
+ /StructParents 0
+>>
+endobj
+{{object 4 0}} <<
+ {{streamlen}}
+>>
+stream
+0.1 w
+/Artifact
+BMC
+q
+0 0 612 792 re
+W* n
+EMC
+/Figure<</MCID 0>>
+BDC
+Q
+q
+281 685.3 50 50 re
+W* n
+q
+49.9 0 0 50 281.1 685.4 cm
+/Im7 Do
+Q
+EMC
+Q
+q
+EGS9 gs /Tr8 Do
+Q
+endstream
+endobj
+{{object 5 0}} <<
+ /Type /XObject
+ /Subtype /Form
+ /BBox [-140 395 753 395.1]
+ /Group <<
+ /CS /DeviceRGB
+ /K true
+ /S /Transparency
+ >>
+ {{streamlen}}
+>>
+stream
+endstream
+endobj
+{{object 6 0}} <<
+ /Type /XObject
+ /Subtype /Image
+ /Width 50
+ /Height 50
+ /BitsPerComponent 8
+ /ColorSpace /DeviceRGB
+ /Filter [/ASCIIHexDecode /FlateDecode]
+ {{streamlen}}
+>>
+stream
+789cedc13101000000c2a0f54fed6f06a00000000000000078031d4c0001
+endstream
+endobj
+{{object 7 0}} <<
+ /ca 0.5
+ /CA 0.5
+>>
+endobj
+{{object 8 0}} <<
+ /Type /StructTreeRoot
+ /ParentTree 9 0 R
+ /K [10 0 R]
+ /RoleMap <<
+ /Document /Document
+ /Standard /P
+ /Figure /Figure
+ >>
+>>
+endobj
+{{object 9 0}} <<
+ /Nums [
+ 0
+ [10 0 R 11 0 R 12 0 R 13 0 R 14 0 R 15 0 R 16 0 R 17 0 R]
+ ]
+>>
+endobj
+{{object 10 0}} <<
+ /Type /StructElem
+ /S /Document
+ /K [11 0 R]
+ /P 8 0 R
+ /T (TitleText)
+ /Pg 3 0 R
+ /Lang (en-US)
+>>
+endobj
+{{object 11 0}} <<
+ /Type /StructElem
+ /S /Table
+ /K [12 0 R] % Deliberately left off object 13 0.
+ /P 10 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /Summary ()
+ >>]
+ /ID (node12)
+ /Lang (hu)
+>>
+endobj
+{{object 12 0}} <<
+ /Type /StructElem
+ /S /TR
+ /K [14 0 R 15 0 R]
+ /P 11 0 R
+ /Pg 3 0 R
+ /ID ()
+>>
+endobj
+{{object 13 0}} <<
+ /Type /StructElem
+ /S /TR
+ /K [16 0 R] % Deliberately left off object 17 0.
+ /P 11 0 R
+ /Pg 3 0 R
+ /A <<
+ /O /Table
+ >>
+ /ID (node14)
+>>
+endobj
+{{object 14 0}} <<
+ /Type /StructElem
+ /S /TH
+ /P 12 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /Scope /Row
+ >>
+ <<
+ /O /Table
+ /ColSpan 2
+ >>]
+ /ID (node15)
+>>
+endobj
+{{object 15 0}} <<
+ /Type /StructElem
+ /S /TD
+ /P 12 0 R
+ /Pg 3 0 R
+ /ID (node16)
+>>
+endobj
+{{object 16 0}} <<
+ /Type /StructElem
+ /S /TH
+ /P 13 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /Scope /Row
+ >>]
+ /ID (node17)
+>>
+endobj
+{{object 17 0}} <<
+ /Type /StructElem
+ /S /TD
+ /P 13 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /ColProp (Sum)
+ /CurUSD true
+ >>]
+ /ID (node18)
+>>
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/tagged_table_bad_parent.pdf b/testing/resources/tagged_table_bad_parent.pdf
new file mode 100644
index 0000000..43d9aa3
--- /dev/null
+++ b/testing/resources/tagged_table_bad_parent.pdf
@@ -0,0 +1,241 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /StructTreeRoot 8 0 R
+ /Lang (en-US)
+ /MarkInfo <<
+ /Marked true
+ >>
+>>
+endobj
+2 0 obj <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /Contents 4 0 R
+ /MediaBox [0 0 612 792]
+ /Group <<
+ /CS /DeviceRGB
+ /I true
+ /S /Transparency
+ >>
+ /Resources <<
+ /ProcSet [/PDF /ImageC /ImageI /ImageB]
+ /XObject <<
+ /Tr8 5 0 R
+ /Im7 6 0 R
+ >>
+ /ExtGState <<
+ /EGS9 7 0 R
+ >>
+ >>
+ /StructParents 0
+>>
+endobj
+4 0 obj <<
+ /Length 162
+>>
+stream
+0.1 w
+/Artifact
+BMC
+q
+0 0 612 792 re
+W* n
+EMC
+/Figure<</MCID 0>>
+BDC
+Q
+q
+281 685.3 50 50 re
+W* n
+q
+49.9 0 0 50 281.1 685.4 cm
+/Im7 Do
+Q
+EMC
+Q
+q
+EGS9 gs /Tr8 Do
+Q
+endstream
+endobj
+5 0 obj <<
+ /Type /XObject
+ /Subtype /Form
+ /BBox [-140 395 753 395.1]
+ /Group <<
+ /CS /DeviceRGB
+ /K true
+ /S /Transparency
+ >>
+ /Length 0
+>>
+stream
+endstream
+endobj
+6 0 obj <<
+ /Type /XObject
+ /Subtype /Image
+ /Width 50
+ /Height 50
+ /BitsPerComponent 8
+ /ColorSpace /DeviceRGB
+ /Filter [/ASCIIHexDecode /FlateDecode]
+ /Length 61
+>>
+stream
+789cedc13101000000c2a0f54fed6f06a00000000000000078031d4c0001
+endstream
+endobj
+7 0 obj <<
+ /ca 0.5
+ /CA 0.5
+>>
+endobj
+8 0 obj <<
+ /Type /StructTreeRoot
+ /ParentTree 9 0 R
+ /K [10 0 R]
+ /RoleMap <<
+ /Document /Document
+ /Standard /P
+ /Figure /Figure
+ >>
+>>
+endobj
+9 0 obj <<
+ /Nums [
+ 0
+ [10 0 R 11 0 R 12 0 R 13 0 R 14 0 R 15 0 R 16 0 R 17 0 R]
+ ]
+>>
+endobj
+10 0 obj <<
+ /Type /StructElem
+ /S /Document
+ /K [11 0 R]
+ /P 8 0 R
+ /T (TitleText)
+ /Pg 3 0 R
+ /Lang (en-US)
+>>
+endobj
+11 0 obj <<
+ /Type /StructElem
+ /S /Table
+ /K [12 0 R] % Deliberately left off object 13 0.
+ /P 10 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /Summary ()
+ >>]
+ /ID (node12)
+ /Lang (hu)
+>>
+endobj
+12 0 obj <<
+ /Type /StructElem
+ /S /TR
+ /K [14 0 R 15 0 R]
+ /P 11 0 R
+ /Pg 3 0 R
+ /ID ()
+>>
+endobj
+13 0 obj <<
+ /Type /StructElem
+ /S /TR
+ /K [16 0 R] % Deliberately left off object 17 0.
+ /P 11 0 R
+ /Pg 3 0 R
+ /A <<
+ /O /Table
+ >>
+ /ID (node14)
+>>
+endobj
+14 0 obj <<
+ /Type /StructElem
+ /S /TH
+ /P 12 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /Scope /Row
+ >>
+ <<
+ /O /Table
+ /ColSpan 2
+ >>]
+ /ID (node15)
+>>
+endobj
+15 0 obj <<
+ /Type /StructElem
+ /S /TD
+ /P 12 0 R
+ /Pg 3 0 R
+ /ID (node16)
+>>
+endobj
+16 0 obj <<
+ /Type /StructElem
+ /S /TH
+ /P 13 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /Scope /Row
+ >>]
+ /ID (node17)
+>>
+endobj
+17 0 obj <<
+ /Type /StructElem
+ /S /TD
+ /P 13 0 R
+ /Pg 3 0 R
+ /A [<<
+ /O /Table
+ /ColProp (Sum)
+ /CurUSD true
+ >>]
+ /ID (node18)
+>>
+endobj
+xref
+0 18
+0000000000 65535 f
+0000000015 00000 n
+0000000145 00000 n
+0000000208 00000 n
+0000000556 00000 n
+0000000770 00000 n
+0000000952 00000 n
+0000001212 00000 n
+0000001253 00000 n
+0000001412 00000 n
+0000001515 00000 n
+0000001642 00000 n
+0000001856 00000 n
+0000001961 00000 n
+0000002134 00000 n
+0000002336 00000 n
+0000002426 00000 n
+0000002573 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 18
+>>
+startxref
+2743
+%%EOF
