The default bug view has changed. See this FAQ.
Bug 914017 (CVE-2013-5604)

Stack-buffer-overflow in txXPathNodeUtils::getBaseURI

RESOLVED FIXED in Firefox 25, Firefox OS v1.1hd

Status

()

Core
XSLT
--
critical
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Abhishek Arya, Assigned: peterv)

Tracking

(4 keywords)

Trunk
mozilla27
x86_64
All
crash, csectype-uninitialized, sec-high, testcase
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox25 verified, firefox26 verified, firefox27 verified, firefox-esr17 verified, firefox-esr24 verified, b2g18 fixed, b2g-v1.1hd fixed, b2g-v1.2 fixed)

Details

(Whiteboard: [asan][adv-main25+][adv-esr1710+][adv-esr24-1+])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 801365 [details]
Testcase

==12693==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff3757e180 at pc 0x7ffc253f35bf bp 0x7fff3757dbe0 sp 0x7fff3757dbd8
READ of size 8 at 0x7fff3757e180 thread T0
    #0 0x7ffc253f35be in txXPathNodeUtils::getBaseURI(txXPathNode const&, nsAString_internal&) content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp:569
    #1 0x7ffc25419cc0 in txLoadedDocumentsHash::~txLoadedDocumentsHash() content/xslt/src/xslt/txExecutionState.cpp:39
    #2 0x7ffc2541aa77 in txExecutionState::~txExecutionState() content/xslt/src/xslt/txExecutionState.cpp:94
    #3 0x7ffc2545273c in txMozillaXSLTProcessor::TransformToDoc(nsIDOMDocument**, bool) content/xslt/src/xslt/txMozillaXSLTProcessor.cpp:679
    #4 0x7ffc2804cfc1 in NS_InvokeByIndex xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
    #5 0x7ffc26242ece in Invoke js/xpconnect/src/XPCWrappedNative.cpp:2809
    #6 0x7ffc26242ece in CallMethodHelper js/xpconnect/src/XPCWrappedNative.cpp:2149
    #7 0x7ffc26242ece in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) js/xpconnect/src/XPCWrappedNative.cpp:2115
    #8 0x7ffc2625506b in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1310
    #9 0x7ffc2984ee0a in native js/src/jscntxtinlines.h:219
    #10 0x7ffc2984ee0a in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:489
    #11 0x7ffc29842069 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2459
    #12 0x7ffc298309f1 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:446
    #13 0x7ffc2985108b in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:630
    #14 0x7ffc298513e6 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:666
    #15 0x7ffc29a54c6d in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) js/src/jsapi.cpp:5120
    #16 0x7ffc256b72ef in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions&, JS::Value*) dom/base/nsJSUtils.cpp:268
    #17 0x7ffc256a5357 in nsJSContext::EvaluateString(nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, bool, JS::Value*) dom/base/nsJSEnvironment.cpp:993
    #18 0x7ffc25662dfc in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) dom/base/nsGlobalWindow.cpp:10473
    #19 0x7ffc256476d2 in nsGlobalWindow::RunTimeout(nsTimeout*) dom/base/nsGlobalWindow.cpp:10717
    #20 0x7ffc2566203c in nsGlobalWindow::TimerCallback(nsITimer*, void*) dom/base/nsGlobalWindow.cpp:10964
    #21 0x7ffc2800f9e1 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:546
    #22 0x7ffc28010086 in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:630
    #23 0x7ffc28005b39 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:622
    #24 0x7ffc27f2d571 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan/xpcom/build/nsThreadUtils.cpp:238
    #25 0x7ffc26d38880 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:116
    #26 0x7ffc28125253 in RunInternal ipc/chromium/src/base/message_loop.cc:220
    #27 0x7ffc28125253 in RunHandler ipc/chromium/src/base/message_loop.cc:213
    #28 0x7ffc28125253 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:187
    #29 0x7ffc26b2201c in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:161
    #30 0x7ffc265101ce in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:269
    #31 0x7ffc2392f300 in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3869
    #32 0x7ffc2393023a in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3937
    #33 0x7ffc2393116b in XRE_main toolkit/xre/nsAppRunner.cpp:4139
    #34 0x45524d in do_main browser/app/nsBrowserApp.cpp:275
    #35 0x45524d in main browser/app/nsBrowserApp.cpp:635
    #36 0x7ffc32a9676c in
    #37 0x45483c in _start
Address 0x7fff3757e180 is located in stack of thread T0 at offset 896 in frame
    #0 0x7ffc2545214f in txMozillaXSLTProcessor::TransformToDoc(nsIDOMDocument**, bool) content/xslt/src/xslt/txMozillaXSLTProcessor.cpp:629
  This frame has 4 object(s):
    [32, 40) 'sourceDOMDocument'
    [96, 784) 'es'
    [832, 872) 'handlerFactory'
    [928, 936) 'doc'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
Shadow bytes around the buggy address:
  0x100066ea7be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100066ea7bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100066ea7c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100066ea7c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100066ea7c20: 00 00 f4 f4 f2 f2 f2 f2 00 00 00 00 00 f4 f4 f4
=>0x100066ea7c30:[f2]f2 f2 f2 00 f4 f4 f4 f3 f3 f3 f3 00 00 00 00
  0x100066ea7c40: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x100066ea7c50: 00 00 00 00 00 00 f4 f4 f2 f2 f2 f2 00 00 00 00
  0x100066ea7c60: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
  0x100066ea7c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100066ea7c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Heap right redzone:    fb
  Freed heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==12693==ABORTING

Updated

4 years ago
Severity: normal → critical
Component: General → XSLT
Keywords: crash, testcase
Product: Firefox → Core
Whiteboard: [asan]
Flags: sec-bounty?
Could please you take a look at this, Peter?
Flags: needinfo?(peterv)
(Assignee)

Comment 2

4 years ago
Created attachment 810505 [details] [diff] [review]
v1

If txExecutionState::init failed before it called txLoadedDocumentsHash::init we left the mSourceDocument uninitialized, and then tried to use it from the destructor. This patch also makes txLoadedDocumentsHash::init infallible because nsTHashtable is too.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Attachment #810505 - Flags: review?(jonas)
Flags: needinfo?(peterv)
Attachment #810505 - Flags: review?(jonas) → review+
Flags: sec-bounty? → sec-bounty+
Keywords: csec-uninitialized, sec-high
Please get sec-approval before checking in, assuming this isn't trunk only. How far back does this issue go? Does it affect ESR17?
Peter, is this ready to get sec-approval, land, etc.?  Thanks.
Flags: needinfo?(peterv)
(Assignee)

Comment 6

4 years ago
Comment on attachment 810505 [details] [diff] [review]
v1

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

Not very easily I think, if we don't check in the testcase.

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

The testcase does, so we should probably hold off on checking it in?

Which older supported branches are affected by this flaw?

All AFAIK.

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

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

Not yet, but they won't be hard. Some parts of the patch rely on the fact that certain operations are infallible which they aren't on the branches, so they'd need some additional error checking.

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

Unlikely to cause regressions, it just makes sure we don't access an unitialized pointer from a destructor.
Attachment #810505 - Flags: sec-approval?
Please prepare branch patches (including for ESR17 and 24 unless that isn't feasible) and nominate them.

Also, we need the tests to be a separate patch that can be checked in later. Once these are done, I'll approve. We're close enough to the line that I don't want to approve this for trunk and then get it punted for Beta (aka 25) because of lack of time until release.
(Assignee)

Comment 8

4 years ago
Created attachment 817781 [details] [diff] [review]
v1

[Security approval request comment]
See comment 6.

Turns out we don't need separate patches for branches, all of them have the same infallible hashtables.
Attachment #810505 - Attachment is obsolete: true
Attachment #810505 - Flags: sec-approval?
Attachment #817781 - Flags: sec-approval?
Attachment #817781 - Flags: review+
Flags: needinfo?(peterv)
(Assignee)

Comment 9

4 years ago
Created attachment 817782 [details] [diff] [review]
Automated testcase v1
Attachment #817782 - Flags: review+
Comment on attachment 817781 [details] [diff] [review]
v1

sec-approval+ for trunk. I've approved this for both ESR branches, Aurora, and Beta but it needs to go in as soon as possible since we're running out of beta builds.
Attachment #817781 - Flags: sec-approval?
Attachment #817781 - Flags: sec-approval+
Attachment #817781 - Flags: approval-mozilla-esr24+
Attachment #817781 - Flags: approval-mozilla-esr17+
Attachment #817781 - Flags: approval-mozilla-beta+
Attachment #817781 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 11

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e282e8a9712a
fixed on central https://hg.mozilla.org/mozilla-central/rev/e282e8a9712a
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox27: --- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
https://hg.mozilla.org/releases/mozilla-aurora/rev/cdee8d489101
https://hg.mozilla.org/releases/mozilla-beta/rev/e449862b8ddd
https://hg.mozilla.org/releases/mozilla-esr24/rev/108b5be7dd02
https://hg.mozilla.org/releases/mozilla-b2g18/rev/bb1ba0f59b31
https://hg.mozilla.org/releases/mozilla-esr17/rev/1ee27c334366
status-b2g18: --- → fixed
status-b2g-v1.1hd: --- → affected
status-b2g-v1.2: --- → fixed
status-firefox25: --- → fixed
status-firefox26: --- → fixed
status-firefox-esr17: --- → fixed
status-firefox-esr24: --- → fixed
Whiteboard: [asan] → [asan][adv-main25+][adv-esr1710+][adv-esr24-1+]
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/05db0f8744fa
status-b2g-v1.1hd: affected → fixed
Alias: CVE-2013-5604
Confirmed bug FF27, 2013-09-25.
Verified fixed 17esr, 24esr, 25, 26, 27, 2013-10-21.
status-firefox25: fixed → verified
status-firefox26: fixed → verified
status-firefox27: fixed → verified
status-firefox-esr17: fixed → verified
status-firefox-esr24: fixed → verified
Group: core-security
You need to log in before you can comment on or make changes to this bug.