Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REGRESSION (277924@main): nullptr deref crash calling XSLTProcessor.transformToFragment() before parsing XML #28147

Conversation

ddkilzer
Copy link
Contributor

@ddkilzer ddkilzer commented May 4, 2024

cbbffda

REGRESSION (277924@main): nullptr deref crash calling XSLTProcessor.transformToFragment() 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

37eccf4

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
  πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-skia
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ api-gtk
βœ… πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge loading πŸ›  watch-sim

@ddkilzer ddkilzer requested a review from cdumez as a code owner May 4, 2024 18:07
@ddkilzer ddkilzer self-assigned this May 4, 2024
@ddkilzer ddkilzer added the XML For bugs in XML support other than XML DOM, including XSLT, XMLHTTPRequest, XML Parsing, etc. label May 4, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 4, 2024
@ddkilzer ddkilzer removed the merging-blocked Applied to prevent a change from being merged label May 5, 2024
@ddkilzer ddkilzer force-pushed the eng/REGRESSION-277924main-nullptr-deref-crash-calling-XSLTProcessor-transformToFragment-before-parsing-XML branch from e9a39ea to e357c91 Compare May 5, 2024 01:54
@ddkilzer ddkilzer force-pushed the eng/REGRESSION-277924main-nullptr-deref-crash-calling-XSLTProcessor-transformToFragment-before-parsing-XML branch from e357c91 to 37eccf4 Compare May 6, 2024 18:06
@ddkilzer ddkilzer added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 6, 2024
…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-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-277924main-nullptr-deref-crash-calling-XSLTProcessor-transformToFragment-before-parsing-XML branch from 37eccf4 to cbbffda Compare May 6, 2024 20:12
@webkit-commit-queue
Copy link
Collaborator

Committed 278419@main (cbbffda): https://commits.webkit.org/278419@main

Reviewed commits have been landed. Closing PR #28147 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit cbbffda into WebKit:main May 6, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 6, 2024
@ddkilzer ddkilzer deleted the eng/REGRESSION-277924main-nullptr-deref-crash-calling-XSLTProcessor-transformToFragment-before-parsing-XML branch May 6, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
XML For bugs in XML support other than XML DOM, including XSLT, XMLHTTPRequest, XML Parsing, etc.
Projects
None yet
5 participants