Closed Bug 1936613 (CVE-2025-1009) Opened 8 months ago Closed 7 months ago

Firefox: use-after-free in txMozillaXSLTProcessor

Categories

(Core :: XSLT, defect)

Product:
Core
Component:
XSLT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
136 Branch
Tracking Flags:
Tracking Status
firefox-esr115 135+ verified
firefox-esr128 135+ verified
firefox133 --- wontfix
firefox134 --- wontfix
firefox135 + verified
firefox136 + verified

People

(Reporter: ifratric, Assigned: mccr8)

References

Details

(Keywords: csectype-uaf, reporter-external, sec-high, Whiteboard: [Disclosure deadline 2025-03-11][adv-main135+][adv-ESR115.20+][adv-ESR128.7+])

Attachments

(7 files)

xsltpoc.html
987 bytes, text/html
Details
Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.
48 bytes, text/x-phabricator-request
tjr
: sec-approval+
Details | Review
Bug 1936613 - Crash test.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-beta+
Details | Review
Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-esr128+
Details | Review
Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-esr115+
Details | Review
advisory.txt
192 bytes, text/plain
Details
Attached file xsltpoc.html —

Steps to reproduce:

Please note:

This bug is subject to a 90-day disclosure deadline. If a fix for this
issue is made available to users before the end of the 90-day deadline,
this bug report will become public 30 days after the fix was made
available. Otherwise, this bug report will become public at the deadline.
The scheduled deadline is 2025-03-11.

For more details, see the Project Zero vulnerability disclosure policy:
https://googleprojectzero.blogspot.com/p/vulnerability-disclosure-policy.html

For the discovery of the issue, please credit Ivan Fratric of Google Project Zero

There is a use-after-free vulnerability in Firefox in txMozillaXSLTProcessor. The vulnerability was reproduced with an ASAN build from the latest source code.

The PoC is attached. The correspondin ASAN log can be found at the end of this report. Root cause analysis follows.

txMozillaXSLTProcessor contains several fields that are relevant for understanding the vulnerability:

class txMozillaXSLTProcessor final : public nsIDocumentTransformer,
                                     public nsStubMutationObserver,
                                     public nsWrapperCache {                                   
  ...
  RefPtr<txStylesheet> mStylesheet;
  mozilla::dom::Document* mStylesheetDocument;  // weak
  nsCOMPtr<nsIContent> mEmbeddedStylesheetRoot;
  ...
  nsresult mCompileResult;
};

mStylesheet is the compiled stylesheet, mStylesheetDocument is the document that contains the stylesheet and, if a node is passed to XSLTProcessor.importStylesheet rather than a document, then mEmbeddedStylesheetRoot contains a pointer to that node. mCompileResult contains the compilation result but is only set by some of the functions that perform compilation (we'll come back to that later).

Most notably, mStylesheetDocument is the raw pointer to the Document object. In order to prevent cases where the Document gets freed but is still referenced by txMozillaXSLTProcessor, txMozillaXSLTProcessor registers itself as a MutationObserver and implements the following function, which will be called before the document gets freed:

void txMozillaXSLTProcessor::NodeWillBeDestroyed(nsINode* aNode) {
  nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
  if (NS_FAILED(mCompileResult)) {
    return;
  }
  mCompileResult = ensureStylesheet();
  mStylesheetDocument = nullptr;
  mEmbeddedStylesheetRoot = nullptr;
}

As can be seen, before the document gets deleted, mStylesheetDocument gets set to nullptr. However, we only proceed to that point if not NS_FAILED(mCompileResult). And we can only set mCompileResult to failed from txMozillaXSLTProcessor::NodeWillBeDestroyed, at which point the mStylesheetDocument has already been set to nullptr.

But what if we can set mStylesheetDocument to another document after mCompileResult became FAILED. This would prevent the mutation observer from correctly executing for the second document.

Normally, we are not allowed to set mStylesheetDocument after it has been set previously. txMozillaXSLTProcessor::ImportStylesheet is meant to prevent that:

void txMozillaXSLTProcessor::ImportStylesheet(nsINode& aStyle,
                                              mozilla::ErrorResult& aRv) {
  // We don't support importing multiple stylesheets yet.
  if (NS_WARN_IF(mStylesheetDocument || mStylesheet)) {
    aRv.Throw(NS_ERROR_NOT_IMPLEMENTED);
    return;
  }
  ...
};

As can be seen, txMozillaXSLTProcessor::ImportStylesheet will fail if either mStylesheetDocument or mStylesheet are set. But we already saw that we can unset mStylesheetDocument in txMozillaXSLTProcessor::NodeWillBeDestroyed observer callback. And mStylesheet can also be unset by other observer callbacks, for example:

void txMozillaXSLTProcessor::ContentWillBeRemoved(nsIContent* aChild) {
  mStylesheet = nullptr;
}

So by using observer callbacks, we can get txMozillaXSLTProcessor to a state where both mStylesheetDocument and mStylesheet are NULL, but mCompileResult is FAILED.

Another hurdle to overcome before triggering the bug is that mCompileResult can only be set to FAILED during NodeWillBeDestroyed callback. But at this point, the document being destroyed is empty and the compilation of the empty document will actually succeed. To overcome this, we also set the mEmbeddedStylesheetRoot property (by calling txMozillaXSLTProcessor::ImportStylesheet on a node rather than a document), but we later detatch mEmbeddedStylesheetRoot from mStylesheetDocument using Document.adoptNode so that mStylesheetDocument could be freed (otherwise, mEmbeddedStylesheetRoot would hold a reference to mStylesheetDocument which would prevent freeing).

The entire exploit flow goes somewhat like this:

  1. We call txMozillaXSLTProcessor::ImportStylesheet on a node. All of mStylesheet, mStylesheetDocument and mEmbeddedStylesheetRoot get set.
  2. We detatch mEmbeddedStylesheetRoot from mStylesheetDocument by calling adoptNode on a different document.
  3. We change the content of mEmbeddedStylesheetRoot so that compilation fails the next time. This also result in one of the observer callbacks setting mStylesheet to NULL.
  4. We delete all other references to mStylesheetDocument. Garbage collector runs. txMozillaXSLTProcessor::NodeWillBeDestroyed runs. mCompileResult becomes FAILED, while mStylesheetDocument and mEmbeddedStylesheetRoot become NULL.
  5. We call txMozillaXSLTProcessor::ImportStylesheet with a new document.
  6. We drop all other references to the new document. txMozillaXSLTProcessor::NodeWillBeDestroyed runs but returns early due to NS_FAILED(mCompileResult). Document gets deleted and mStylesheetDocument points to freed memoy.
  7. We call txMozillaXSLTProcessor::Reset. mStylesheetDocument->RemoveMutationObserver(this); is called with mStylesheetDocument being freed, resulting in a use-after-free.

ASAN log:

=================================================================
==766789==ERROR: AddressSanitizer: heap-use-after-free on address 0x51e0000180e0 at pc 0x7f0504a31b6e bp 0x7ffdba15e4a0 sp 0x7ffdba15e498
READ of size 8 at 0x51e0000180e0 thread T0 (file:// Content)
    #0 0x7f0504a31b6d in GetExistingSlots firefox-source/dom/base/nsINode.h:2413:46
    #1 0x7f0504a31b6d in RemoveMutationObserver firefox-source/dom/base/nsINode.h:1276:18
    #2 0x7f0504a31b6d in txMozillaXSLTProcessor::Reset() firefox-source/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:881:26
    #3 0x7f04ff4b4fc8 in mozilla::dom::XSLTProcessor_Binding::reset(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) firefox-source/objdir-ff-asan/dom/bindings/./XSLTProcessorBinding.cpp:1344:24
    #4 0x7f04ffe530ea in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) firefox-source/dom/bindings/BindingUtils.cpp:3290:13
    #5 0x7f050783480f in CallJSNative firefox-source/js/src/vm/Interpreter.cpp:532:13
    #6 0x7f050783480f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) firefox-source/js/src/vm/Interpreter.cpp:628:12
    #7 0x7f050784e27b in InternalCall firefox-source/js/src/vm/Interpreter.cpp:695:10
    #8 0x7f050784e27b in CallFromStack firefox-source/js/src/vm/Interpreter.cpp:700:10
    #9 0x7f050784e27b in js::Interpret(JSContext*, js::RunState&) firefox-source/js/src/vm/Interpreter.cpp:3338:16
    #10 0x7f05078335ce in MaybeEnterInterpreterTrampoline firefox-source/js/src/vm/Interpreter.cpp:433:10
    #11 0x7f05078335ce in js::RunScript(JSContext*, js::RunState&) firefox-source/js/src/vm/Interpreter.cpp:502:13
    #12 0x7f050783878f in ExecuteKernel firefox-source/js/src/vm/Interpreter.cpp:893:13
    #13 0x7f050783878f in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) firefox-source/js/src/vm/Interpreter.cpp:926:10
    #14 0x7f05079a5ba0 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) firefox-source/js/src/vm/CompilationAndEvaluation.cpp:601:10
    #15 0x7f05079a5e60 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) firefox-source/js/src/vm/CompilationAndEvaluation.cpp:625:10
    #16 0x7f0504c951c7 in ExecuteCompiledScript firefox-source/dom/script/ScriptLoader.cpp:2653:8
    #17 0x7f0504c951c7 in mozilla::dom::ScriptLoader::EvaluateScript(nsIGlobalObject*, JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:3137:7
    #18 0x7f0504c93f08 in mozilla::dom::ScriptLoader::EvaluateScriptElement(JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:2722:10
    #19 0x7f0504c8c524 in mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:2354:10
    #20 0x7f0504c89523 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, JS::loader::ScriptKind) firefox-source/dom/script/ScriptLoader.cpp:1605:10
    #21 0x7f0504c71660 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) firefox-source/dom/script/ScriptLoader.cpp:1201:10
    #22 0x7f0504c70f72 in mozilla::dom::ScriptElement::MaybeProcessScript() firefox-source/dom/script/ScriptElement.cpp:210:18
    #23 0x7f04fb36fdc6 in AttemptToExecute firefox-source/objdir-ff-asan/dist/include/nsIScriptElement.h:224:18
    #24 0x7f04fb36fdc6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*, bool) firefox-source/parser/html/nsHtml5TreeOpExecutor.cpp:900:22
    #25 0x7f04fb36b85a in nsHtml5TreeOpExecutor::RunFlushLoop() firefox-source/parser/html/nsHtml5TreeOpExecutor.cpp:685:7
    #26 0x7f04fb3afccb in nsHtml5ExecutorFlusher::Run() firefox-source/parser/html/nsHtml5StreamParser.cpp:161:18
    #27 0x7f04f911ff0a in mozilla::RunnableTask::Run() firefox-source/xpcom/threads/TaskController.cpp:688:16
    #28 0x7f04f90fff3d in mozilla::TaskController::RunTask(mozilla::Task*) firefox-source/xpcom/threads/TaskController.cpp:215:19
    #29 0x7f04f9106ea0 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) firefox-source/xpcom/threads/TaskController.cpp:1015:20
    #30 0x7f04f91048a8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) firefox-source/xpcom/threads/TaskController.cpp:838:15
    #31 0x7f04f9104ec6 in mozilla::TaskController::ProcessPendingMTTask(bool) firefox-source/xpcom/threads/TaskController.cpp:624:36
    #32 0x7f04f910ea51 in operator() firefox-source/xpcom/threads/TaskController.cpp:336:37
    #33 0x7f04f910ea51 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() firefox-source/xpcom/threads/nsThreadUtils.h:548:5
    #34 0x7f04f91502bf in nsThread::ProcessNextEvent(bool, bool*) firefox-source/xpcom/threads/nsThread.cpp:1159:16
    #35 0x7f04f915a6c0 in NS_ProcessNextEvent(nsIThread*, bool) firefox-source/xpcom/threads/nsThreadUtils.cpp:480:10
    #36 0x7f04faae72ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox-source/ipc/glue/MessagePump.cpp:85:21
    #37 0x7f04fa92ac94 in RunInternal firefox-source/ipc/chromium/src/base/message_loop.cc:369:10
    #38 0x7f04fa92ac94 in RunHandler firefox-source/ipc/chromium/src/base/message_loop.cc:362:3
    #39 0x7f04fa92ac94 in MessageLoop::Run() firefox-source/ipc/chromium/src/base/message_loop.cc:344:3
    #40 0x7f0505188ccc in nsBaseAppShell::Run() firefox-source/widget/nsBaseAppShell.cpp:148:27
    #41 0x7f050536096c in nsAppShell::Run() firefox-source/widget/gtk/nsAppShell.cpp:469:33
    #42 0x7f05074312f5 in XRE_RunAppShell() firefox-source/toolkit/xre/nsEmbedFunctions.cpp:646:20
    #43 0x7f04fa92ac94 in RunInternal firefox-source/ipc/chromium/src/base/message_loop.cc:369:10
    #44 0x7f04fa92ac94 in RunHandler firefox-source/ipc/chromium/src/base/message_loop.cc:362:3
    #45 0x7f04fa92ac94 in MessageLoop::Run() firefox-source/ipc/chromium/src/base/message_loop.cc:344:3
    #46 0x7f0507430b82 in XRE_InitChildProcess(int, char**, XREChildData const*) firefox-source/toolkit/xre/nsEmbedFunctions.cpp:584:34
    #47 0x55c511826849 in main firefox-source/browser/app/nsBrowserApp.cpp:397:22
    #48 0x7f0517840c89 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #49 0x7f0517840d44 in __libc_start_main csu/../csu/libc-start.c:360:3
    #50 0x55c511745ef8 in _start (firefox-source/objdir-ff-asan/dist/bin/firefox+0xccef8) (BuildId: 113c3f167b1e04b8ec5edf06452c0bb9)

0x51e0000180e0 is located 96 bytes inside of 2808-byte region [0x51e000018080,0x51e000018b78)
freed by thread T0 (file:// Content) here:
    #0 0x55c5117e3556 in free /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
    #1 0x7f04f8f7e1c1 in MaybeKillObject firefox-source/xpcom/base/nsCycleCollector.cpp:2625:29
    #2 0x7f04f8f7e1c1 in SnowWhiteKiller::~SnowWhiteKiller() firefox-source/xpcom/base/nsCycleCollector.cpp:2612:7
    #3 0x7f04f8f65aef in nsCycleCollector::FreeSnowWhite(bool) firefox-source/xpcom/base/nsCycleCollector.cpp:2802:3
    #4 0x7f04f8f6affb in nsCycleCollector::CleanupAfterCollection() firefox-source/xpcom/base/nsCycleCollector.cpp:3501:3
    #5 0x7f04f8f6c18b in nsCycleCollector::Collect(mozilla::CCReason, ccIsManual, JS::SliceBudget&, nsICycleCollectorListener*, bool) firefox-source/xpcom/base/nsCycleCollector.cpp:3653:9
    #6 0x7f04f8f70379 in nsCycleCollector_collect(mozilla::CCReason, nsICycleCollectorListener*) firefox-source/xpcom/base/nsCycleCollector.cpp:4154:28
    #7 0x7f04fdceacc3 in nsJSContext::CycleCollectNow(mozilla::CCReason, nsICycleCollectorListener*) firefox-source/dom/base/nsJSEnvironment.cpp:1110:3
    #8 0x7f04ffb23308 in mozilla::dom::FuzzingFunctions_Binding::cycleCollect(JSContext*, unsigned int, JS::Value*) firefox-source/objdir-ff-asan/dom/bindings/./FuzzingFunctionsBinding.cpp:133:3
    #9 0x7f050783480f in CallJSNative firefox-source/js/src/vm/Interpreter.cpp:532:13
    #10 0x7f050783480f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) firefox-source/js/src/vm/Interpreter.cpp:628:12
    #11 0x7f050784e27b in InternalCall firefox-source/js/src/vm/Interpreter.cpp:695:10
    #12 0x7f050784e27b in CallFromStack firefox-source/js/src/vm/Interpreter.cpp:700:10
    #13 0x7f050784e27b in js::Interpret(JSContext*, js::RunState&) firefox-source/js/src/vm/Interpreter.cpp:3338:16
    #14 0x7f05078335ce in MaybeEnterInterpreterTrampoline firefox-source/js/src/vm/Interpreter.cpp:433:10
    #15 0x7f05078335ce in js::RunScript(JSContext*, js::RunState&) firefox-source/js/src/vm/Interpreter.cpp:502:13
    #16 0x7f050783878f in ExecuteKernel firefox-source/js/src/vm/Interpreter.cpp:893:13
    #17 0x7f050783878f in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) firefox-source/js/src/vm/Interpreter.cpp:926:10
    #18 0x7f05079a5ba0 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) firefox-source/js/src/vm/CompilationAndEvaluation.cpp:601:10
    #19 0x7f05079a5e60 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) firefox-source/js/src/vm/CompilationAndEvaluation.cpp:625:10
    #20 0x7f0504c951c7 in ExecuteCompiledScript firefox-source/dom/script/ScriptLoader.cpp:2653:8
    #21 0x7f0504c951c7 in mozilla::dom::ScriptLoader::EvaluateScript(nsIGlobalObject*, JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:3137:7
    #22 0x7f0504c93f08 in mozilla::dom::ScriptLoader::EvaluateScriptElement(JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:2722:10
    #23 0x7f0504c8c524 in mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:2354:10
    #24 0x7f0504c89523 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, JS::loader::ScriptKind) firefox-source/dom/script/ScriptLoader.cpp:1605:10
    #25 0x7f0504c71660 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) firefox-source/dom/script/ScriptLoader.cpp:1201:10
    #26 0x7f0504c70f72 in mozilla::dom::ScriptElement::MaybeProcessScript() firefox-source/dom/script/ScriptElement.cpp:210:18
    #27 0x7f04fb36fdc6 in AttemptToExecute firefox-source/objdir-ff-asan/dist/include/nsIScriptElement.h:224:18
    #28 0x7f04fb36fdc6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*, bool) firefox-source/parser/html/nsHtml5TreeOpExecutor.cpp:900:22
    #29 0x7f04fb36b85a in nsHtml5TreeOpExecutor::RunFlushLoop() firefox-source/parser/html/nsHtml5TreeOpExecutor.cpp:685:7
    #30 0x7f04fb3afccb in nsHtml5ExecutorFlusher::Run() firefox-source/parser/html/nsHtml5StreamParser.cpp:161:18
    #31 0x7f04f911ff0a in mozilla::RunnableTask::Run() firefox-source/xpcom/threads/TaskController.cpp:688:16
    #32 0x7f04f90fff3d in mozilla::TaskController::RunTask(mozilla::Task*) firefox-source/xpcom/threads/TaskController.cpp:215:19
    #33 0x7f04f9106ea0 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) firefox-source/xpcom/threads/TaskController.cpp:1015:20
    #34 0x7f04f91048a8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) firefox-source/xpcom/threads/TaskController.cpp:838:15
    #35 0x7f04f9104ec6 in mozilla::TaskController::ProcessPendingMTTask(bool) firefox-source/xpcom/threads/TaskController.cpp:624:36
    #36 0x7f04f910ea51 in operator() firefox-source/xpcom/threads/TaskController.cpp:336:37
    #37 0x7f04f910ea51 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() firefox-source/xpcom/threads/nsThreadUtils.h:548:5
    #38 0x7f04f91502bf in nsThread::ProcessNextEvent(bool, bool*) firefox-source/xpcom/threads/nsThread.cpp:1159:16

previously allocated by thread T0 (file:// Content) here:
    #0 0x55c5117e37ef in malloc /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:68:3
    #1 0x55c51182a88d in moz_xmalloc firefox-source/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f050499fc04 in operator new firefox-source/objdir-ff-asan/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7f050499fc04 in operator new firefox-source/objdir-ff-asan/dist/include/mozilla/dom/Document.h:566:45
    #4 0x7f050499fc04 in NS_NewXMLDocument firefox-source/dom/xml/XMLDocument.cpp:179:29
    #5 0x7f050499fc04 in NS_NewDOMDocument(mozilla::dom::Document**, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::dom::DocumentType*, nsIURI*, nsIURI*, nsIPrincipal*, bool, nsIGlobalObject*, DocumentFlavor) firefox-source/dom/xml/XMLDocument.cpp:74:10
    #6 0x7f04fd732f77 in mozilla::dom::DOMImplementation::CreateDocument(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::dom::DocumentType*, mozilla::dom::Document**) firefox-source/dom/base/DOMImplementation.cpp:100:8
    #7 0x7f04fd7332bc in mozilla::dom::DOMImplementation::CreateDocument(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::dom::DocumentType*, mozilla::ErrorResult&) firefox-source/dom/base/DOMImplementation.cpp:128:9
    #8 0x7f04ff73af91 in mozilla::dom::DOMImplementation_Binding::createDocument(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) firefox-source/objdir-ff-asan/dom/bindings/./DOMImplementationBinding.cpp:162:75
    #9 0x7f04ffe530ea in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) firefox-source/dom/bindings/BindingUtils.cpp:3290:13
    #10 0x7f050783480f in CallJSNative firefox-source/js/src/vm/Interpreter.cpp:532:13
    #11 0x7f050783480f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) firefox-source/js/src/vm/Interpreter.cpp:628:12
    #12 0x7f050784e27b in InternalCall firefox-source/js/src/vm/Interpreter.cpp:695:10
    #13 0x7f050784e27b in CallFromStack firefox-source/js/src/vm/Interpreter.cpp:700:10
    #14 0x7f050784e27b in js::Interpret(JSContext*, js::RunState&) firefox-source/js/src/vm/Interpreter.cpp:3338:16
    #15 0x7f05078335ce in MaybeEnterInterpreterTrampoline firefox-source/js/src/vm/Interpreter.cpp:433:10
    #16 0x7f05078335ce in js::RunScript(JSContext*, js::RunState&) firefox-source/js/src/vm/Interpreter.cpp:502:13
    #17 0x7f050783878f in ExecuteKernel firefox-source/js/src/vm/Interpreter.cpp:893:13
    #18 0x7f050783878f in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) firefox-source/js/src/vm/Interpreter.cpp:926:10
    #19 0x7f05079a5ba0 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) firefox-source/js/src/vm/CompilationAndEvaluation.cpp:601:10
    #20 0x7f05079a5e60 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) firefox-source/js/src/vm/CompilationAndEvaluation.cpp:625:10
    #21 0x7f0504c951c7 in ExecuteCompiledScript firefox-source/dom/script/ScriptLoader.cpp:2653:8
    #22 0x7f0504c951c7 in mozilla::dom::ScriptLoader::EvaluateScript(nsIGlobalObject*, JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:3137:7
    #23 0x7f0504c93f08 in mozilla::dom::ScriptLoader::EvaluateScriptElement(JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:2722:10
    #24 0x7f0504c8c524 in mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*) firefox-source/dom/script/ScriptLoader.cpp:2354:10
    #25 0x7f0504c89523 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, JS::loader::ScriptKind) firefox-source/dom/script/ScriptLoader.cpp:1605:10
    #26 0x7f0504c71660 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) firefox-source/dom/script/ScriptLoader.cpp:1201:10
    #27 0x7f0504c70f72 in mozilla::dom::ScriptElement::MaybeProcessScript() firefox-source/dom/script/ScriptElement.cpp:210:18
    #28 0x7f04fb36fdc6 in AttemptToExecute firefox-source/objdir-ff-asan/dist/include/nsIScriptElement.h:224:18
    #29 0x7f04fb36fdc6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*, bool) firefox-source/parser/html/nsHtml5TreeOpExecutor.cpp:900:22
    #30 0x7f04fb36b85a in nsHtml5TreeOpExecutor::RunFlushLoop() firefox-source/parser/html/nsHtml5TreeOpExecutor.cpp:685:7
    #31 0x7f04fb3afccb in nsHtml5ExecutorFlusher::Run() firefox-source/parser/html/nsHtml5StreamParser.cpp:161:18
    #32 0x7f04f911ff0a in mozilla::RunnableTask::Run() firefox-source/xpcom/threads/TaskController.cpp:688:16
    #33 0x7f04f90fff3d in mozilla::TaskController::RunTask(mozilla::Task*) firefox-source/xpcom/threads/TaskController.cpp:215:19
    #34 0x7f04f9106ea0 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) firefox-source/xpcom/threads/TaskController.cpp:1015:20
    #35 0x7f04f91048a8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) firefox-source/xpcom/threads/TaskController.cpp:838:15
    #36 0x7f04f9104ec6 in mozilla::TaskController::ProcessPendingMTTask(bool) firefox-source/xpcom/threads/TaskController.cpp:624:36
    #37 0x7f04f910ea51 in operator() firefox-source/xpcom/threads/TaskController.cpp:336:37
    #38 0x7f04f910ea51 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() firefox-source/xpcom/threads/nsThreadUtils.h:548:5
    #39 0x7f04f91502bf in nsThread::ProcessNextEvent(bool, bool*) firefox-source/xpcom/threads/nsThread.cpp:1159:16
    #40 0x7f04f915a6c0 in NS_ProcessNextEvent(nsIThread*, bool) firefox-source/xpcom/threads/nsThreadUtils.cpp:480:10

SUMMARY: AddressSanitizer: heap-use-after-free firefox-source/dom/base/nsINode.h:2413:46 in GetExistingSlots
Shadow bytes around the buggy address:
  0x51e000017e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x51e000017e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x51e000017f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
  0x51e000017f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x51e000018000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x51e000018080: fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd
  0x51e000018100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51e000018180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51e000018200: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51e000018280: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51e000018300: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==766789==ABORTING
Group: firefox-core-security → core-security
Component: Untriaged → XSLT
Product: Firefox → Core
Group: core-security → dom-core-security
Flags: needinfo?(peterv)

I've updated the first comment to clean up the markup a little.

Severity: -- → S2

The most obvious fix here is to change mStylesheetDocument to a WeakPtr. I confirmed that this patch stops the UAF. I don't know if we should address any of the other weird inconsistencies that the POC relies on.

At a glance, there are no other raw-pointer-as-weak-pointers in the XSLT implementation, or at least not any documented as such. We should really go through and try to get rid of all of them in DOM code...

Let's just fix the UAF here. Peter can look into the other weirdness described here if he wants later.

Assignee: nobody → continuation

Unfortunately this is not a full fix. I confirmed the fix when loading the test case independently. However, when I converted it to a crash test and ran it alongside the other XSLT crash tests, I hit another UAF.

We're in the nsINode::LastRelease() for a document and calling NodeWillBeDestroyed. I'm guessing the issue is that when we do the adoptNode and then the original document dies, the newly WeakPtr'd mStylesheetDocument safely turns into null. Hooray! But then in ~txMozillaXSLTProcessor() this means we never call RemoveMutationObserver, so the new document has a dangling pointer. Oops. I guess I need to actually fix this issue on a deeper level.

The new UAF is actually in a different test 1527277.html, which just hits an error. I suspect the problem is something like: Document gets unlinked (clearing the weak reference), txMozillaXSLTProcessor gets destroyed (weak ref is null, so it can't RemoveMutationObserver itself), Document gets destroyed (triggering the observer on the dead object). Back to the drawing board I guess.

Attachment #9443103 - Attachment description: Bug 1936613 - Use WeakPtr in txMozillaXSLTProcessor. → Bug 1936613 - Don't early return from txMozillaXSLTProcessor::NodeWillBeDestroyed.

Maybe txMozillaXSLTProcessor::ImportStylesheet can call txMozillaXSLTProcessor::Reset first? I'm not sure why mStylesheetDocument is weak though, but if the Reset thing works we might want to do that in a followup.

Flags: needinfo?(continuation)

Thanks for taking a look. I thought I tried making mStylesheetDocument and it leaked, but I tried it again now and I didn't see a leak. Maybe I forgot to make it CC'ed the first time? Anyways unfortunately making it strong causes browser_bug1309630.js to time out, though now I'm not seeing any leaks. I was mostly avoiding it because it could cause a bunch of leakiness that might be hard to find. I'll try the reset thing.

Today is the last day for uplifts to beta 134, so given the uncertainty here, it'll have to wait until next release cycle. I think this dates to bug 199331 which landed in 2003 so this can probably wait a few more weeks.

status-firefox133: --- → wontfix
status-firefox134: --- → wontfix
status-firefox135: --- → affected
status-firefox-esr115: --- → affected
status-firefox-esr128: --- → affected

(In reply to Peter Van der Beken [:peterv] from comment #8)

Maybe txMozillaXSLTProcessor::ImportStylesheet can call txMozillaXSLTProcessor::Reset first? I'm not sure why mStylesheetDocument is weak though, but if the Reset thing works we might want to do that in a followup.

That does seem to also fix the issue. Doesn't that change the behavior in the case where you do an import twice, though? Is that not a problem?

Flags: needinfo?(continuation)

I also audited the other implementations of NodeWillBeDestroyed and I didn't notice any others that did this weird early return thing.

(In reply to Andrew McCreight [:mccr8] from comment #11)

That does seem to also fix the issue. Doesn't that change the behavior in the case where you do an import twice, though? Is that not a problem?

Hmm, I guess the main problem is that then the parameters/variables would also be cleared. I had an alternative approach in https://phabricator.services.mozilla.com/D231880#8036293, up to you.

Flags: needinfo?(peterv)
Attachment #9443103 - Attachment description: Bug 1936613 - Don't early return from txMozillaXSLTProcessor::NodeWillBeDestroyed. → Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.
Blocks: 1937634

I filed bug 1937634 about making this reference strong. The browser_bug1309630.js timeout I mentioned was a preexisting issue, but I'm still a little nervous about whether making it a strong ref will cause a leak.

Whiteboard: [Disclosure deadline 2025-03-11]

Comment on attachment 9443103 [details]
Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The steps involved are a bit convoluted, but you might be able to work backwards to figure out how to cause the UAF. This bug is 21 years old AFAICT.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: Shouldn't be an issue. This is a small, local fix to a file that doesn't change much.
  • How likely is this patch to cause regressions; how much testing does it need?: Not likely. I don't think it'll affect behavior except in the case where the exploit is being triggered.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: Yes
Attachment #9443103 - Flags: sec-approval?

Comment on attachment 9443103 [details]
Bug 1936613 - Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet.

Approved to land and uplift

Attachment #9443103 - Flags: sec-approval? → sec-approval+
Whiteboard: [Disclosure deadline 2025-03-11] → [Disclosure deadline 2025-03-11][reminder-test 2025-02-18]
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9d746bbe4f0a Reset mCompileResult in txMozillaXSLTProcessor::ImportStylesheet. r=peterv

https://hg.mozilla.org/mozilla-central/rev/9d746bbe4f0a

Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 7 months ago
status-firefox136: --- → fixed
tracking-firefox136: --- → +
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch

The patch landed in nightly and beta is affected.
:mccr8, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox135 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(continuation)
Attachment #9446568 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: sec-high
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: Load the xsltpoc.html attachment, see if it crashes after a few seconds
  • Risk associated with taking this patch: low
  • Explanation of risk level: this should only change the behavior in a weird case where we'd crash anyways
  • String changes made/needed: none
  • Is Android affected?: yes
Flags: qe-verify+

I checked that the attachment doesn't crash for me on MacOS Nightly, but it would still be good to have a real verification done.

Attachment #9446571 - Flags: approval-mozilla-esr128?

esr128 Uplift Approval Request

  • User impact if declined: sec-high
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: Load the xsltpoc.html attachment, see if it crashes after a few seconds
  • Risk associated with taking this patch: low
  • Explanation of risk level: this should only change the behavior in a weird case where we'd crash anyways
  • String changes made/needed: none
  • Is Android affected?: yes
Attachment #9446572 - Flags: approval-mozilla-esr115?

esr115 Uplift Approval Request

  • User impact if declined: sec-high
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: Load the xsltpoc.html attachment, see if it crashes after a few seconds
  • Risk associated with taking this patch: low
  • Explanation of risk level: this should only change the behavior in a weird case where we'd crash anyways
  • String changes made/needed: none
  • Is Android affected?: yes
Flags: needinfo?(continuation)
Attachment #9446568 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [post-critsmash-triage]
QA Whiteboard: [post-critsmash-triage] → [post-critsmash-triage] [qa-triaged]
Attachment #9446571 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
Attachment #9446572 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+

Reproduced the issue on Ubuntu 24.04. by following the next steps:

  1. Download Firefox (2024-12-11) ASAN fuzzing build with fuzzfetch: fuzzfetch -a --fuzzing --build 2024-12-11
  2. Run Grizzly replay with the xsltpoc.html testcase downloaded locally: python3 -m grizzly.replay '/home/svuser/m-c-20241211100250-fuzzing-asan-opt/firefox' '/home/svuser/Downloads/xsltpoc.html'

AR: Firefox closes and the Grizzly console reports Result successfully reproduced: AddressSanitizer: heap-use-after-free [@ GetExistingSlots] with READ of size 8 (6386aab7:b9cb1e5b)

The issue no longer occurs (no results detected) on Ubuntu 24.04 by following the above steps with ASAN fuzzing Firefox 136.0a1 (20250112212231), 135.0b4 (20250112225912), 128.7.0esr (20250112190713), 115.20.0esr (20250110165353- downloaded manually from link). If any further verification is needed, please let us know.

Status: RESOLVED → VERIFIED
status-firefox135: fixed → verified
status-firefox136: fixed → verified
status-firefox-esr115: fixed → verified
status-firefox-esr128: fixed → verified
Flags: qe-verify+

Apropos of nothing, why is this patch adding debug assert and not a release assert?

Flags: needinfo?(continuation)
Flags: needinfo?(continuation) → needinfo?(peterv)

Because mEmbeddedStylesheetRoot should be reset when mStylesheet is reset (which we do). Failing to do so would not be dramatic, it might lead to weird behaviour though. What exactly would you be worried about?

Flags: needinfo?(peterv)
Flags: needinfo?(fbraun)

Not worried about anything in particular. It just caught my eye as odd from a high level, when a security patch is adding an assertion that isn't in release :)

Flags: needinfo?(fbraun)
Whiteboard: [Disclosure deadline 2025-03-11][reminder-test 2025-02-18] → [Disclosure deadline 2025-03-11][reminder-test 2025-02-18][adv-main135+]
Whiteboard: [Disclosure deadline 2025-03-11][reminder-test 2025-02-18][adv-main135+] → [Disclosure deadline 2025-03-11][reminder-test 2025-02-18][adv-main135+][adv-ESR115.20+][adv-ESR128.7+]
Attached file advisory.txt —
Alias: CVE-2025-1009

a month ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2025-02-18] .

mccr8, please refer to the original comment to better understand the reason for the reminder.

Flags: needinfo?(continuation)
Whiteboard: [Disclosure deadline 2025-03-11][reminder-test 2025-02-18][adv-main135+][adv-ESR115.20+][adv-ESR128.7+] → [Disclosure deadline 2025-03-11][adv-main135+][adv-ESR115.20+][adv-ESR128.7+]
Flags: needinfo?(continuation)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: