REGRESSION (277924@main): nullptr deref crash calling XSLTProcessor.t…

…ransformToFragment() before parsing XML <https://bugs.webkit.org/show_bug.cgi?id=273735> <rdar://127496002> Reviewed by Alex Christensen. If docLoaderFunc() in XSLTProcessorLibxslt.cpp was called before an XML document was parsed, the WebCore::defaultEntityLoader global would not be initialized, which could result in a nullptr dereference crash. The fix is to call initializeXMLParser() in XMLDocumentParserScope() constructors since there are cases where XMLDocumentParserScope is used but XMLParserContext (the only place where initializeXMLParser() was called previously) is not. Test: fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash.html * LayoutTests/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash-expected.txt: Add. * LayoutTests/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash.html: Add. - Test is marked "runSingly=true" since parsing any XML content before running the test avoids the crash. * LayoutTests/platform/glib/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash-expected.txt: Add. - Platform-specific results for GTK and WPE ports. * Source/WebCore/xml/parser/XMLDocumentParser.h: (WebCore::initializeXMLParser): Add declaration. * Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp: (WebCore::externalEntityLoader): - Add RELEASE_ASSERT() for the cause of the original crash. (WebCore::initializeXMLParser): - Remove static keyword so this can be called from XMLDocumentParserScope() constructors. * Source/WebCore/xml/parser/XMLDocumentParserScope.cpp: (WebCore::XMLDocumentParserScope::XMLDocumentParserScope): - Call initializeXMLParser() from constructors before setting m_oldEntityLoader. Canonical link: https://commits.webkit.org/278419@main
WebKit · May 6, 2024 · cbbffda · cbbffda
1 parent 0995cf2
commit cbbffda
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 3 deletions.
diff --git a/LayoutTests/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash-expected.txt b/LayoutTests/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash-expected.txt
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: Did not parse external entity resource at '' because cross-origin loads are not allowed.
+CONSOLE MESSAGE: Start tag expected, '<' not found
+
+PASS if no crash
diff --git a/LayoutTests/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash.html b/LayoutTests/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html><!-- webkit-test-runner [ runSingly=true ] -->
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+function test()
+{
+    processor = new XSLTProcessor();
+    docType = document.implementation.createDocumentType("xml:test", "-//test//Test 1.0//EN", "M/");
+    processor.importStylesheet(docType);
+    processor.transformToFragment(node1, document);
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</head>
+<body onload="test()">
+<span id="node1">PASS if no crash</span>
+</body>
+</html>
diff --git a/...Tests/platform/glib/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash-expected.txt b/...Tests/platform/glib/fast/xsl/xslt-transform-to-fragment-no-xml-parsing-crash-expected.txt
@@ -0,0 +1,6 @@
+CONSOLE MESSAGE: Document is empty
+
+CONSOLE MESSAGE: Did not parse external entity resource at '' because cross-origin loads are not allowed.
+CONSOLE MESSAGE: Start tag expected, '<' not found
+
+PASS if no crash
diff --git a/Source/WebCore/xml/parser/XMLDocumentParser.h b/Source/WebCore/xml/parser/XMLDocumentParser.h
@@ -193,6 +193,7 @@ xmlDocPtr xmlDocPtrForString(CachedResourceLoader&, const String& source, const
 #endif
 
 xmlParserInputPtr externalEntityLoader(const char* url, const char* id, xmlParserCtxtPtr);
+void initializeXMLParser();
 
 std::optional<HashMap<String, String>> parseAttributes(CachedResourceLoader&, const String&);
 

diff --git a/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp b/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp
@@ -553,10 +553,11 @@ xmlParserInputPtr externalEntityLoader(const char* url, const char* id, xmlParse
 {
     if (!shouldAllowExternalLoad(URL(String::fromUTF8(url))))
         return nullptr;
+    RELEASE_ASSERT_WITH_MESSAGE(!!defaultEntityLoader, "Missing call to initializeXMLParser()");
     return defaultEntityLoader(url, id, context);
 }
 
-static void initializeXMLParser()
+void initializeXMLParser()
 {
     static std::once_flag flag;
     std::call_once(flag, [&] {

diff --git a/Source/WebCore/xml/parser/XMLDocumentParserScope.cpp b/Source/WebCore/xml/parser/XMLDocumentParserScope.cpp
@@ -45,8 +45,9 @@ XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResou
 #else
 XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResourceLoader)
     : m_oldCachedResourceLoader(currentCachedResourceLoader())
-    , m_oldEntityLoader(xmlGetExternalEntityLoader())
 {
+    initializeXMLParser();
+    m_oldEntityLoader = xmlGetExternalEntityLoader();
     currentCachedResourceLoader() = cachedResourceLoader;
     xmlSetExternalEntityLoader(WebCore::externalEntityLoader);
 }
@@ -55,11 +56,12 @@ XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResou
 #if ENABLE(XSLT)
 XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResourceLoader, xmlGenericErrorFunc genericErrorFunc, xmlStructuredErrorFunc structuredErrorFunc, void* errorContext)
     : m_oldCachedResourceLoader(currentCachedResourceLoader())
-    , m_oldEntityLoader(xmlGetExternalEntityLoader())
     , m_oldGenericErrorFunc(xmlGenericError)
     , m_oldStructuredErrorFunc(xmlStructuredError)
     , m_oldErrorContext(xmlGenericErrorContext)
 {
+    initializeXMLParser();
+    m_oldEntityLoader = xmlGetExternalEntityLoader();
     currentCachedResourceLoader() = cachedResourceLoader;
     xmlSetExternalEntityLoader(WebCore::externalEntityLoader);
     if (genericErrorFunc)