commit 7f2235ee95ce49835b5850cd4819b4a7f1705653
author Hui Yingst <nigi@chromium.org> Wed Jun 21 21:21:49 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jun 21 21:21:49 2023 +0000
tree 3075d9d178fd168654cdbe93017b5dd59b6afbb0
parent 4d1379a8d47c8d441d4d3076fe712d0815cf1786

Fix stack-overflow in CheckForSharedFormInternal()

CheckForSharedFormInternal() is called recursively to traverse the
nodes in a meta data tree. If the meta data contains corrupted data
and never closes XML tags correctly, it could trigger stack-overflow
in this function.

This CL adds `kMaxMetaDataDepth` as the arbitrary maximum meta data
tree depth to prevent too many recursive calls of
CheckForSharedFormInternal() and adds a unit test for it. Also
renames `pElement` to `xml_element` to fit the c++ style guide better.

Bug: chromium:1012377
Change-Id: Iaad7a07c6dc17d37130fdfd1e02441ba50b2066e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/108910
Commit-Queue: Nigi <nigi@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

core/fpdfdoc/cpdf_metadata.cpp

diff --git a/core/fpdfdoc/cpdf_metadata.cpp b/core/fpdfdoc/cpdf_metadata.cpp
index 228a0c1..31564f4 100644
--- a/core/fpdfdoc/cpdf_metadata.cpp
+++ b/core/fpdfdoc/cpdf_metadata.cpp
@@ -20,8 +20,15 @@
 
 namespace {
 
-void CheckForSharedFormInternal(CFX_XMLElement* element,
+constexpr int kMaxMetaDataDepth = 128;
+
+bool CheckForSharedFormInternal(int depth,
+                                CFX_XMLElement* element,
                                 std::vector<UnsupportedFeature>* unsupported) {
+  if (depth >= kMaxMetaDataDepth) {
+    return false;
+  }
+
   WideString attr =
       element->GetAttribute(WideString::FromASCII("xmlns:adhocwf"));
   if (attr.EqualsASCII("http://ns.adobe.com/AcrobatAdhocWorkflow/1.0/")) {
@@ -54,10 +61,13 @@
 
   for (auto* child = element->GetFirstChild(); child;
        child = child->GetNextSibling()) {
-    CFX_XMLElement* pElement = ToXMLElement(child);
-    if (pElement)
-      CheckForSharedFormInternal(pElement, unsupported);
+    CFX_XMLElement* xml_element = ToXMLElement(child);
+    if (xml_element &&
+        !CheckForSharedFormInternal(depth + 1, xml_element, unsupported)) {
+      return false;
+    }
   }
+  return true;
 }
 
 }  // namespace
@@ -80,6 +90,6 @@
     return {};
 
   std::vector<UnsupportedFeature> unsupported;
-  CheckForSharedFormInternal(doc->GetRoot(), &unsupported);
+  CheckForSharedFormInternal(/*depth=*/0, doc->GetRoot(), &unsupported);
   return unsupported;
 }

core/fpdfdoc/cpdf_metadata_unittest.cpp

diff --git a/core/fpdfdoc/cpdf_metadata_unittest.cpp b/core/fpdfdoc/cpdf_metadata_unittest.cpp
index cfcb8f3..4bd30ed 100644
--- a/core/fpdfdoc/cpdf_metadata_unittest.cpp
+++ b/core/fpdfdoc/cpdf_metadata_unittest.cpp
@@ -106,6 +106,39 @@
   EXPECT_EQ(0U, results.size());
 }
 
+TEST(CPDF_MetadataTest, CheckSharedFormExceedMaxDepth) {
+  // Node <parent> has the depth of 130, which exceeds the maximum node depth of
+  // `kMaxMetaDataDepth`.
+  static const char kData[] =
+      "<?xml charset=\"utf-8\"?>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<node><node><node><node><node><node><node><node><node><node>\n"
+      "<parent>\n"
+      "<node xmlns:adhocwf=\"http://ns.adobe.com/AcrobatAdhocWorkflow/1.0/\">\n"
+      "<adhocwf:workflowType>0</adhocwf:workflowType>\n"
+      "<adhocwf:version>1.1</adhocwf:version>\n"
+      "</node>"
+      "</parent>";
+
+  auto stream = pdfium::MakeRetain<CPDF_Stream>();
+  stream->SetData(ByteStringView(kData).raw_span());
+  CPDF_Metadata metadata(stream);
+
+  auto results = metadata.CheckForSharedForm();
+  ASSERT_EQ(0U, results.size());
+}
+
 TEST(CPDF_MetadataTest, CheckSharedFormWrongNamespace) {
   static const char data[] =
       "<?xml charset=\"utf-8\"?>\n"