Closed Bug 1944313 (CVE-2025-1932) Opened 1 year ago Closed 1 year ago

Firefox: inconsistent comparator in xslt/txNodeSorter leads to out-of-bounds access

Categories

(Core :: XSLT, defect)

Product:
Core
Component:
XSLT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
137 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 136+ verified
firefox134 --- wontfix
firefox135 --- wontfix
firefox136 + verified
firefox137 --- verified

People

(Reporter: ifratric, Assigned: peterv, NeedInfo)

References

Details

(4 keywords, Whiteboard: [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed][adv-main136+][adv-esr128.8+])

Attachments

(4 files)

sort.html
871 bytes, text/html
Details
Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!
48 bytes, text/x-phabricator-request
dmeehan
: approval-mozilla-beta+
dmeehan
: approval-mozilla-esr128+
tjr
: sec-approval+
Details | Review
Bug 1944313 - Testcase. r?farre!
48 bytes, text/x-phabricator-request
Details | Review
advisory.txt
261 bytes, text/plain
Details
Attached file sort.html —

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36

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-04-28.

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.

Details:

In C++, when using sorting algorithms with a custom comparator, a comparator needs to be consistent and always return the same output for the same input. Otherwise, it's possible to access data out-of bounds.

In Firefox's XSLT implementation, the txNodeSorter::compareNodes comparator is not state free: In particular, if aSortData.mRv ever failes (e.g. due to createSortableValue() failing in txNodeSorter::calcSortValue), then any future call to txNodeSorter::compareNodes will return -1, which follows from:

int txNodeSorter::compareNodes(uint32_t aIndexA, uint32_t aIndexB,
                               SortData& aSortData) {
  NS_ENSURE_SUCCESS(aSortData.mRv, -1);
  ...
}

Because of this, std::stable_sort can access out-of bounds data.

A proof of concept is attached (reproduced with an ASAN build from the latest source code).

While the proof of concept results in an ASAN report on out-of-bounds read, I believe this issue can result in more than just reads if out-of-bounds data ends up in the "sorted" array, whose elements are then used as indices when constructing the resulting node list.

ASAN log:

=================================================================
==2937759==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5030000ab7ec at pc 0x7f5e4d13d429 bp 0x7ffc0918d800 sp 0x7ffc0918d7f8
READ of size 4 at 0x5030000ab7ec thread T0 (file:// Content)
    #0 0x7f5e4d13d428 in Compare<const unsigned int, const unsigned int> objdir-ff-asan/dist/include/nsTArray.h:930:31
    #1 0x7f5e4d13d428 in LessThan<const unsigned int, const unsigned int> objdir-ff-asan/dist/include/nsTArray.h:940:12
    #2 0x7f5e4d13d428 in operator()<unsigned int, unsigned int> objdir-ff-asan/dist/include/nsTArray.h:2397:36
    #3 0x7f5e4d13d428 in operator()<unsigned int, unsigned int *> .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/predefined_ops.h:215:16
    #4 0x7f5e4d13d428 in __unguarded_linear_insert<unsigned int *, __gnu_cxx::__ops::_Val_comp_iter<(lambda at objdir-ff-asan/dist/include/nsTArray.h:2396:22)> > .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:1828:14
    #5 0x7f5e4d13d428 in void std::__insertion_sort<unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<void nsTArray_Impl<unsigned int, nsTArrayInfallibleAllocator>::StableSort<txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&)::'lambda'(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&, auto const&)>>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0, txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0, auto) .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:1855:6
    #6 0x7f5e4d13dbdf in __chunk_insertion_sort<unsigned int *, long, __gnu_cxx::__ops::_Iter_comp_iter<(lambda at objdir-ff-asan/dist/include/nsTArray.h:2396:22)> > .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:2698:7
    #7 0x7f5e4d13dbdf in void std::__merge_sort_with_buffer<unsigned int*, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<void nsTArray_Impl<unsigned int, nsTArrayInfallibleAllocator>::StableSort<txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&)::'lambda'(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&, auto const&)>>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0, txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0, auto, __gnu_cxx::__ops::_Iter_comp_iter<void nsTArray_Impl<unsigned int, nsTArrayInfallibleAllocator>::StableSort<txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&)::'lambda'(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&, auto const&)>) .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:2716:7
    #8 0x7f5e4d13d0cb in void std::__stable_sort_adaptive<unsigned int*, unsigned int*, long, __gnu_cxx::__ops::_Iter_comp_iter<void nsTArray_Impl<unsigned int, nsTArrayInfallibleAllocator>::StableSort<txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&)::'lambda'(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&, auto const&)>>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0, txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0, auto, long, __gnu_cxx::__ops::_Iter_comp_iter<void nsTArray_Impl<unsigned int, nsTArrayInfallibleAllocator>::StableSort<txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0>(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&)::'lambda'(txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**)::$_0 const&, auto const&)>) .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:2749:4
    #9 0x7f5e4d118714 in __stable_sort<unsigned int *, __gnu_cxx::__ops::_Iter_comp_iter<(lambda at objdir-ff-asan/dist/include/nsTArray.h:2396:22)> > .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:5006:2
    #10 0x7f5e4d118714 in stable_sort<unsigned int *, (lambda at objdir-ff-asan/dist/include/nsTArray.h:2396:22)> .mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_algo.h:5075:7
    #11 0x7f5e4d118714 in StableSort<(lambda at dom/xslt/xslt/txNodeSorter.cpp:158:22)> objdir-ff-asan/dist/include/nsTArray.h:2395:5
    #12 0x7f5e4d118714 in txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**) dom/xslt/xslt/txNodeSorter.cpp:158:11
    #13 0x7f5e4d117173 in txPushNewContext::execute(txExecutionState&) dom/xslt/xslt/txInstructions.cpp:546:15
    #14 0x7f5e4d16e982 in txXSLTProcessor::execute(txExecutionState&) dom/xslt/xslt/txXSLTProcessor.cpp:46:17
    #15 0x7f5e4d13762e in txMozillaXSLTProcessor::TransformToFragment(nsINode&, mozilla::dom::Document&, mozilla::ErrorResult&) dom/xslt/xslt/txMozillaXSLTProcessor.cpp:760:10
    #16 0x7f5e47af9a87 in mozilla::dom::XSLTProcessor_Binding::transformToFragment(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) objdir-ff-asan/dom/bindings/./XSLTProcessorBinding.cpp:1055:83
    #17 0x7f5e484a457a in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp:3290:13
    #18 0x7f5e4ffa00ef in CallJSNative js/src/vm/Interpreter.cpp:532:13
    #19 0x7f5e4ffa00ef in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) js/src/vm/Interpreter.cpp:628:12
    #20 0x7f5e4ffb9b5b in InternalCall js/src/vm/Interpreter.cpp:695:10
    #21 0x7f5e4ffb9b5b in CallFromStack js/src/vm/Interpreter.cpp:700:10
    #22 0x7f5e4ffb9b5b in js::Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3338:16
    #23 0x7f5e4ff9eeae in MaybeEnterInterpreterTrampoline js/src/vm/Interpreter.cpp:433:10
    #24 0x7f5e4ff9eeae in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:502:13
    #25 0x7f5e4ffa406f in ExecuteKernel js/src/vm/Interpreter.cpp:893:13
    #26 0x7f5e4ffa406f in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:926:10
    #27 0x7f5e50111a70 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) js/src/vm/CompilationAndEvaluation.cpp:601:10
    #28 0x7f5e50111d30 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) js/src/vm/CompilationAndEvaluation.cpp:625:10
    #29 0x7f5e4d39c3f7 in ExecuteCompiledScript dom/script/ScriptLoader.cpp:2664:8
    #30 0x7f5e4d39c3f7 in mozilla::dom::ScriptLoader::EvaluateScript(nsIGlobalObject*, JS::loader::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:3148:7
    #31 0x7f5e4d39b138 in mozilla::dom::ScriptLoader::EvaluateScriptElement(JS::loader::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:2733:10
    #32 0x7f5e4d393754 in mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:2365:10
    #33 0x7f5e4d390753 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, JS::loader::ScriptKind) dom/script/ScriptLoader.cpp:1616:10
    #34 0x7f5e4d378780 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) dom/script/ScriptLoader.cpp:1212:10
    #35 0x7f5e4d378092 in mozilla::dom::ScriptElement::MaybeProcessScript() dom/script/ScriptElement.cpp:210:18
    #36 0x7f5e439a85a6 in AttemptToExecute objdir-ff-asan/dist/include/nsIScriptElement.h:224:18
    #37 0x7f5e439a85a6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*, bool) parser/html/nsHtml5TreeOpExecutor.cpp:900:22
    #38 0x7f5e439a403a in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:685:7
    #39 0x7f5e439e84ab in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:161:18
    #40 0x7f5e4173a8ca in mozilla::RunnableTask::Run() xpcom/threads/TaskController.cpp:688:16
    #41 0x7f5e4171a98d in mozilla::TaskController::RunTask(mozilla::Task*) xpcom/threads/TaskController.cpp:215:19
    #42 0x7f5e417218f0 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) xpcom/threads/TaskController.cpp:1015:20
    #43 0x7f5e4171f2f8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) xpcom/threads/TaskController.cpp:838:15
    #44 0x7f5e4171f916 in mozilla::TaskController::ProcessPendingMTTask(bool) xpcom/threads/TaskController.cpp:624:36
    #45 0x7f5e41729441 in operator() xpcom/threads/TaskController.cpp:336:37
    #46 0x7f5e41729441 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() xpcom/threads/nsThreadUtils.h:548:5
    #47 0x7f5e4176ac0f in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1159:16
    #48 0x7f5e41774fe0 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:480:10
    #49 0x7f5e43119adf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:85:21
    #50 0x7f5e42f533f4 in RunInternal ipc/chromium/src/base/message_loop.cc:369:10
    #51 0x7f5e42f533f4 in RunHandler ipc/chromium/src/base/message_loop.cc:362:3
    #52 0x7f5e42f533f4 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:344:3
    #53 0x7f5e4d88f23c in nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:148:27
    #54 0x7f5e4da9769b in nsAppShell::Run() widget/gtk/nsAppShell.cpp:470:33
    #55 0x7f5e4fb9b675 in XRE_RunAppShell() toolkit/xre/nsEmbedFunctions.cpp:646:20
    #56 0x7f5e42f533f4 in RunInternal ipc/chromium/src/base/message_loop.cc:369:10
    #57 0x7f5e42f533f4 in RunHandler ipc/chromium/src/base/message_loop.cc:362:3
    #58 0x7f5e42f533f4 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:344:3
    #59 0x7f5e4fb9af02 in XRE_InitChildProcess(int, char**, XREChildData const*) toolkit/xre/nsEmbedFunctions.cpp:584:34
    #60 0x55f5942cc9a9 in main browser/app/nsBrowserApp.cpp:397:22
    #61 0x7f5e6035ec89 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #62 0x7f5e6035ed44 in __libc_start_main csu/../csu/libc-start.c:360:3
    #63 0x55f5941ec058 in _start (objdir-ff-asan/dist/bin/firefox+0xcd058) (BuildId: a5f6b92e690df25dfe11a977d556c465)

0x5030000ab7ec is located 4 bytes before 24-byte region [0x5030000ab7f0,0x5030000ab808)
allocated by thread T0 (file:// Content) here:
    #0 0x55f59428994f in malloc /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:68:3
    #1 0x55f5942d09ed in moz_xmalloc memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f5e414c2cfa in Malloc objdir-ff-asan/dist/include/nsTArray.h:255:12
    #3 0x7f5e414c2cfa in nsTArrayInfallibleAllocator::ResultTypeProxy nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_RelocateUsingMemutils>::EnsureCapacityImpl<nsTArrayInfallibleAllocator>(unsigned long, unsigned long) objdir-ff-asan/dist/include/nsTArray-inl.h:173:43
    #4 0x7f5e4d118268 in EnsureCapacity<nsTArrayInfallibleAllocator> objdir-ff-asan/dist/include/nsTArray.h:430:12
    #5 0x7f5e4d118268 in SetCapacity<nsTArrayInfallibleAllocator> objdir-ff-asan/dist/include/nsTArray.h:2209:47
    #6 0x7f5e4d118268 in nsTArray_Impl objdir-ff-asan/dist/include/nsTArray.h:1045:49
    #7 0x7f5e4d118268 in nsTArray objdir-ff-asan/dist/include/nsTArray.h:2751:44
    #8 0x7f5e4d118268 in txNodeSorter::sortNodeSet(txNodeSet*, txExecutionState*, txNodeSet**) dom/xslt/xslt/txNodeSorter.cpp:135:22
    #9 0x7f5e4d117173 in txPushNewContext::execute(txExecutionState&) dom/xslt/xslt/txInstructions.cpp:546:15
    #10 0x7f5e4d16e982 in txXSLTProcessor::execute(txExecutionState&) dom/xslt/xslt/txXSLTProcessor.cpp:46:17
    #11 0x7f5e4d13762e in txMozillaXSLTProcessor::TransformToFragment(nsINode&, mozilla::dom::Document&, mozilla::ErrorResult&) dom/xslt/xslt/txMozillaXSLTProcessor.cpp:760:10
    #12 0x7f5e47af9a87 in mozilla::dom::XSLTProcessor_Binding::transformToFragment(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) objdir-ff-asan/dom/bindings/./XSLTProcessorBinding.cpp:1055:83
    #13 0x7f5e484a457a in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp:3290:13
    #14 0x7f5e4ffa00ef in CallJSNative js/src/vm/Interpreter.cpp:532:13
    #15 0x7f5e4ffa00ef in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) js/src/vm/Interpreter.cpp:628:12
    #16 0x7f5e4ffb9b5b in InternalCall js/src/vm/Interpreter.cpp:695:10
    #17 0x7f5e4ffb9b5b in CallFromStack js/src/vm/Interpreter.cpp:700:10
    #18 0x7f5e4ffb9b5b in js::Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3338:16
    #19 0x7f5e4ff9eeae in MaybeEnterInterpreterTrampoline js/src/vm/Interpreter.cpp:433:10
    #20 0x7f5e4ff9eeae in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:502:13
    #21 0x7f5e4ffa406f in ExecuteKernel js/src/vm/Interpreter.cpp:893:13
    #22 0x7f5e4ffa406f in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:926:10
    #23 0x7f5e50111a70 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) js/src/vm/CompilationAndEvaluation.cpp:601:10
    #24 0x7f5e50111d30 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) js/src/vm/CompilationAndEvaluation.cpp:625:10
    #25 0x7f5e4d39c3f7 in ExecuteCompiledScript dom/script/ScriptLoader.cpp:2664:8
    #26 0x7f5e4d39c3f7 in mozilla::dom::ScriptLoader::EvaluateScript(nsIGlobalObject*, JS::loader::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:3148:7
    #27 0x7f5e4d39b138 in mozilla::dom::ScriptLoader::EvaluateScriptElement(JS::loader::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:2733:10
    #28 0x7f5e4d393754 in mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:2365:10
    #29 0x7f5e4d390753 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, JS::loader::ScriptKind) dom/script/ScriptLoader.cpp:1616:10
    #30 0x7f5e4d378780 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) dom/script/ScriptLoader.cpp:1212:10
    #31 0x7f5e4d378092 in mozilla::dom::ScriptElement::MaybeProcessScript() dom/script/ScriptElement.cpp:210:18
    #32 0x7f5e439a85a6 in AttemptToExecute objdir-ff-asan/dist/include/nsIScriptElement.h:224:18
    #33 0x7f5e439a85a6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*, bool) parser/html/nsHtml5TreeOpExecutor.cpp:900:22
    #34 0x7f5e439a403a in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:685:7
    #35 0x7f5e439e84ab in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:161:18
    #36 0x7f5e4173a8ca in mozilla::RunnableTask::Run() xpcom/threads/TaskController.cpp:688:16
    #37 0x7f5e4171a98d in mozilla::TaskController::RunTask(mozilla::Task*) xpcom/threads/TaskController.cpp:215:19
    #38 0x7f5e417218f0 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) xpcom/threads/TaskController.cpp:1015:20
    #39 0x7f5e4171f2f8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) xpcom/threads/TaskController.cpp:838:15
    #40 0x7f5e4171f916 in mozilla::TaskController::ProcessPendingMTTask(bool) xpcom/threads/TaskController.cpp:624:36
    #41 0x7f5e41729441 in operator() xpcom/threads/TaskController.cpp:336:37
    #42 0x7f5e41729441 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() xpcom/threads/nsThreadUtils.h:548:5

SUMMARY: AddressSanitizer: heap-buffer-overflow objdir-ff-asan/dist/include/nsTArray.h:930:31 in Compare<const unsigned int, const unsigned int>
Shadow bytes around the buggy address:
  0x5030000ab500: 00 00 fa fa 00 00 00 00 fa fa 00 00 00 00 fa fa
  0x5030000ab580: 00 00 00 fa fa fa 00 00 00 00 fa fa 00 00 00 00
  0x5030000ab600: fa fa 00 00 00 00 fa fa 00 00 00 00 fa fa 00 00
  0x5030000ab680: 00 00 fa fa 00 00 00 00 fa fa 00 00 00 00 fa fa
  0x5030000ab700: 00 00 00 00 fa fa 00 00 00 fa fa fa 00 00 06 fa
=>0x5030000ab780: fa fa 00 00 00 fa fa fa 00 00 00 fa fa[fa]00 00
  0x5030000ab800: 00 fa fa fa 00 00 00 00 fa fa 00 00 00 00 fa fa
  0x5030000ab880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x5030000ab900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x5030000ab980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x5030000aba00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==2937759==ABORTING
Group: firefox-core-security → dom-core-security
Component: Untriaged → XSLT
Keywords: csectype-bounds, testcase
Product: Firefox → Core
Keywords: sec-high
Whiteboard: [to be disclosed 28 Apr 2025]

I haven't been able to reproduce this yet (with both a Linux and a MacOS ASAN build).

I successfully reproduced the issue using grizzly-replay using the latest available ASan build on Linux.

grizzly-replay <firefox-bin> <testcase>
status-firefox136: --- → affected
Keywords: bugmon
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Attachment #9462379 - Attachment description: Bug 1944313 - Fail early when expressions fail. r?farre! → Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!

Verified bug as reproducible on mozilla-central 20250128161837-d9b57e2b8726.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: 2cac2f68bfdcfc9012c537044bc475f076bda7b0 (20240131050955)
End: 70cabc8df49e79271982f4484a15a2a259d5159b (20250128043739)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False, searchfox=False, afl=False)

Whiteboard: [to be disclosed 28 Apr 2025] → [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed]
Severity: -- → S2

I assume this might qualify as regression of bug 1839051, where I even explicitly touched txNodeSorter::compareNodes when adapting the uses of sort without noting this specific (pre-existing) twist. At least IIRC in other occasions (we fixed) the old quicksort was more tolerant wrt inconsistent comparators.

Comment on attachment 9462379 [details]
Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not sure, requires knowing that unstable compare functions can lead to out-of-bound accesses.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
  • 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?: Should be trivial.
  • How likely is this patch to cause regressions; how much testing does it need?: Unlikely to cause regressions. Behaviour of the APIs shouldn't change, there might be a small performance regression but only in an error case.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: Yes
Attachment #9462379 - Flags: sec-approval?

FYI, this'll need a rebased patch for ESR115.

status-firefox135: affected → wontfix
tracking-firefox136: --- → +
tracking-firefox-esr115: --- → 136+
tracking-firefox-esr128: --- → 136+
Flags: needinfo?(peterv)

I tried running the testcase on an ESR115 ASAN build and couldn't reproduce there. I talked to Jens, since ESR115 doesn't have bug 1839051 and NS_QuickSort might not have the same problem as StableSort we might not need this on ESR115 after all.

Flags: needinfo?(peterv)

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

I tried running the testcase on an ESR115 ASAN build and couldn't reproduce there. I talked to Jens, since ESR115 doesn't have bug 1839051 and NS_QuickSort might not have the same problem as StableSort we might not need this on ESR115 after all.

To expand a bit on that, I remember having seen at least another case where a flaky comparator caused std::sort to fail with out-of-bounds assertions where NS_QuickSort was just happy with delivering a (probably undefined) result. So I assume the worse that can happen on ESR is to end up with a badly sorted array, which is probably irrelevant for this error case, anyways.

Comment on attachment 9462379 [details]
Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!

Approved to land and request uplift

Attachment #9462379 - Flags: sec-approval? → sec-approval+
Whiteboard: [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed] → [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed][reminder-test 2025-04-15]
Pushed by pvanderbeken@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bab87b8804c9 Propagate XSLT expression failure correctly. r=farre

https://hg.mozilla.org/mozilla-central/rev/bab87b8804c9

Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox137: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 137 Branch

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

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

For more information, please visit BugBot documentation.

Flags: needinfo?(peterv)

To add to comment 14, please also add uplift requests for esr115 and esr128

(In reply to Donal Meehan [:dmeehan] from comment #15)

To add to comment 14, please also add uplift requests for esr115 and esr128

I think we concluded that esr115 does not need an uplift.

Verified bug as fixed on rev mozilla-central 20250205095734-5041c20ccabb.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
status-firefox137: fixed → verified
Keywords: bugmon

Comment on attachment 9462379 [details]
Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!

Beta/Release Uplift Approval Request

  • User impact if declined/Reason for urgency: Sec-high.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Loading https://bugzilla.mozilla.org/attachment.cgi?id=9462250 in an ASAN build should not show an error.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It changes behaviour in an error condition to be safe, but the error will just be reported as before.
  • String changes made/needed: None
  • Is Android affected?: Yes

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Sec-high.
  • User impact if declined: Security issue (UAF).
  • Fix Landed on Version: 137
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It changes behaviour in an error condition to be safe, but the error will just be reported as before.
Flags: needinfo?(peterv)
Attachment #9462379 - Flags: approval-mozilla-esr128?
Attachment #9462379 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Resetting the tracking flag for esr115, as we don't think we need to ship it there.

status-firefox-esr115: affected → ?

Comment on attachment 9462379 [details]
Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!

Approved for 136.0b3

Attachment #9462379 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Regressions: 1946636
QA Whiteboard: [qa-triaged]
Regressions: 1946635

Hi, Ivan Fratric! Could you please help us verify this fix on the 136.0b3 Beta? I could not reproduce the issue using the affected Nightly Asan builds (2025-01-28) with the test case from comment 0 or comment 18. I also tried running them with grizzly.replay, but I couldn't trigger any errors on Windows 11 and Ubuntu 22.04.

Flags: needinfo?(ifratric)

Hi Ciprian, I can confirm the latest patch fixes the issue for me. FYI The exact reproducer would depend on the C++ STL version, for Windows, the PoC would have to be modified for the Microsoft STL. Not sure why it wouldn't reproduce on Ubuntu though, because for me the PoC crashes even the release build of Firefox 135.0 (64-bit) for Linux.

Flags: needinfo?(ifratric)

Comment on attachment 9462379 [details]
Bug 1944313 - Propagate XSLT expression failure correctly. r?farre!

Approved for 128.8esr

Attachment #9462379 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

Thanks, Ivan, for checking the fix. I'll mark this verified as fixed on Beta 136 per commment 23.

status-firefox136: fixed → verified
Whiteboard: [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed][reminder-test 2025-04-15] → [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed][reminder-test 2025-04-15][adv-main136+][adv-esr128.8+]

Hi, Ivan Fratric! Can you please do another check of this fix, but this time on 128.8esr?

Flags: needinfo?(ifratric)

Hi Ciprian, I built esr128 from the latest soruce at https://hg.mozilla.org/releases/mozilla-esr128/ and confirmed that the PoC doesn't trigger.

Flags: needinfo?(ifratric)

(In reply to Ivan Fratric from comment #28)

Hi Ciprian, I built esr128 from the latest soruce at https://hg.mozilla.org/releases/mozilla-esr128/ and confirmed that the PoC doesn't trigger.

Great, thank you!

status-firefox-esr128: fixed → verified
Flags: qe-verify+
Alias: CVE-2025-1932
Regressions: 1954841
No longer regressions: 1954841

2 months ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2025-04-15] .

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

Flags: needinfo?(peterv)
Whiteboard: [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed][reminder-test 2025-04-15][adv-main136+][adv-esr128.8+] → [to be disclosed 28 Apr 2025][bugmon:bisected,confirmed][adv-main136+][adv-esr128.8+]
Flags: needinfo?(peterv) → needinfo?(smaug)
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: