commit 8a87da2cf76cdfd1539e30f43ff1add9163a2c1a
author Tom Sepez <tsepez@chromium.org> Tue Mar 07 22:42:23 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Mar 07 22:42:23 2023 +0000
tree eb71ae0d218e58b2709731068fa88f151e4a233e
parent 7ea7c90ea78923e5a347525be6d27531e30d25fe

More tightly validate XML names in CXFA_FFDocView::GetWidgetByName()

Widget names must conform to XML name rules.

-- Beef up tests while at it.

Fixed: chromium:1419831
Change-Id: Id36b4a7b3d84aa0b74d54c91eed2f1a11da8298f
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/104511
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fxcrt/xml/cfx_xmlparser_unittest.cpp

diff --git a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp
index e8019d0..c692a0b 100644
--- a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp
+++ b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp
@@ -290,6 +290,27 @@
   EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'-', true));
   EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'-', false));
 
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'.', true));
+  EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'.', false));
+
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'0', true));
+  EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'0', false));
+
+  EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'a', true));
+  EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'a', false));
+
+  EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'A', true));
+  EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(L'A', false));
+
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'(', false));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'(', true));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L')', false));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L')', true));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'[', false));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L'[', true));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L']', false));
+  EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(L']', true));
+
   EXPECT_FALSE(CFX_XMLParser::IsXMLNameChar(0x2069, true));
   EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(0x2070, true));
   EXPECT_TRUE(CFX_XMLParser::IsXMLNameChar(0x2073, true));

xfa/fxfa/cxfa_ffdocview.cpp

diff --git a/xfa/fxfa/cxfa_ffdocview.cpp b/xfa/fxfa/cxfa_ffdocview.cpp
index 2239cae..7d4ef85 100644
--- a/xfa/fxfa/cxfa_ffdocview.cpp
+++ b/xfa/fxfa/cxfa_ffdocview.cpp
@@ -11,6 +11,7 @@
 
 #include "core/fxcrt/fx_extension.h"
 #include "core/fxcrt/stl_util.h"
+#include "core/fxcrt/xml/cfx_xmlparser.h"
 #include "fxjs/gc/container_trace.h"
 #include "fxjs/xfa/cfxjse_engine.h"
 #include "fxjs/xfa/cjx_object.h"
@@ -43,6 +44,21 @@
 #include "xfa/fxfa/parser/cxfa_validate.h"
 #include "xfa/fxfa/parser/xfa_utils.h"
 
+namespace {
+
+bool IsValidXMLNameString(const WideString& str) {
+  bool first = true;
+  for (const auto ch : str) {
+    if (!CFX_XMLParser::IsXMLNameChar(ch, first)) {
+      return false;
+    }
+    first = false;
+  }
+  return true;
+}
+
+}  // namespace
+
 const XFA_AttributeValue kXFAEventActivity[] = {
     XFA_AttributeValue::Click,      XFA_AttributeValue::Change,
     XFA_AttributeValue::DocClose,   XFA_AttributeValue::DocReady,
@@ -443,6 +459,9 @@
 
 CXFA_FFWidget* CXFA_FFDocView::GetWidgetByName(const WideString& wsName,
                                                CXFA_FFWidget* pRefWidget) {
+  if (!IsValidXMLNameString(wsName)) {
+    return nullptr;
+  }
   CFXJSE_Engine* pScriptContext = m_pDoc->GetXFADoc()->GetScriptContext();
   CXFA_Node* pRefNode = nullptr;
   if (pRefWidget) {