Bug 915210 (CVE-2013-5599)

ASAN heap-use-after-free in nsIPresShell::GetPresContext() with canvas, onresize and mozTextStyle

RESOLVED FIXED in Firefox 25



5 years ago
4 years ago


(Reporter: nils, Assigned: smaug)


({csectype-uaf, sec-critical})

csectype-uaf, sec-critical

Firefox Tracking Flags

(firefox23 wontfix, firefox24 wontfix, firefox25+ verified, firefox26+ verified, firefox27+ verified, firefox-esr1725+ verified, firefox-esr2425+ verified, b2g1825+ fixed, b2g-v1.1hd fixed, b2g-v1.2 fixed)


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


(1 attachment)



5 years ago
The following testcase crashes the firefox ASAN build:

function start() {
o14.src="data:text/html,<body id='e0'></body>";
window.setTimeout('startrly()', 100);
function startrly() {
function cb_frameresize_25_1() {
<body onload="start()">
<div id="s"></div>

Comment 1

5 years ago
The asan output after the crash:

==44288==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160001a7990 at pc 0x7f135250880d bp 0x7fff00d7e250 sp 0x7fff00d7e248
READ of size 8 at 0x6160001a7990 thread T0
    #0 0x7f135250880c in nsIPresShell::GetPresContext() const /layout/style/../base/nsIPresShell.h:286:0
    #1 0x7f13525083b5 in nsComputedDOMStyle::GetStyleContextForElementNoFlush(mozilla::dom::Element*, nsIAtom*, nsIPresShell*, nsComputedDOMStyle::StyleType) /layout/style/nsComputedDOMStyle.cpp:332:54
    #1 0x7f13525083b5 in nsComputedDOMStyle::GetStyleContextForElementNoFlush(mozilla::dom::Element*, nsIAtom*, nsIPresShell*, nsComputedDOMStyle::StyleType) /layout/style/nsComputedDOMStyle.cpp:332:54
    #3 0x7f1352b32822 in mozilla::dom::CanvasRenderingContext2D::SetFont(nsAString_internal const&, mozilla::ErrorResult&) /content/canvas/src/CanvasRenderingContext2D.cpp:2072:0
    #4 0x7f1352b36edb in mozilla::dom::CanvasRenderingContext2D::GetCurrentFontStyle() /content/canvas/src/CanvasRenderingContext2D.cpp:2724:0
    #5 0x7f1354ff7af6 in mozilla::dom::CanvasRenderingContext2D::GetFont() /obj-firefox/dom/bindings/../../dist/include/mozilla/dom/CanvasRenderingContext2D.h:569:0
    #6 0x7f1354ff3c66 in mozilla::dom::CanvasRenderingContext2DBinding::genericGetter(JSContext*, unsigned int, JS::Value*) /obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:4197:0
    #7 0x7f13574b51fa in JSFunction::native() const /js/src/jscntxtinlines.h:219:0
    #8 0x7f13574b6218 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:539:0
    #9 0x7f13574b706c in js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:610:23
    #10 0x7f1357834a53 in js::Shape::get(JSContext*, JS::Handle<JSObject*>, JSObject*, JSObject*, JS::MutableHandle<JS::Value>) /js/src/vm/Shape-inl.h:256:0
    #11 0x7f1357824b63 in _ZL15NativeGetInlineILN2js7AllowGCE1EEbP9JSContextNS0_11MaybeRootedIP8JSObjectXT_EE10HandleTypeES8_S8_NS4_IPNS0_5ShapeEXT_EE10HandleTypeEjNS4_IN2JS5ValueEXT_EE17MutableHandleTypeE /js/src/jsobj.cpp:4070:21
    #12 0x7f13574c722d in GetPropertyOperation(JSContext*, js::StackFrame*, JS::Handle<JSScript*>, unsigned char*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:292:0
    #13 0x7f13574a3122 in Interpret(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:2298:0
    #14 0x7f13574969b1 in js::RunScript(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:446:0
    #15 0x7f13574b74eb in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /js/src/vm/Interpreter.cpp:630:0
    #16 0x7f13574b7846 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /js/src/vm/Interpreter.cpp:666:0
    #17 0x7f13576bcf9d in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) /js/src/jsapi.cpp:5120:0
    #18 0x7f135331870a in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions&, JS::Value*) /dom/base/nsJSUtils.cpp:268:0
    #19 0x7f1353306607 in nsJSContext::EvaluateString(nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, bool, JS::Value*) /dom/base/nsJSEnvironment.cpp:993:0
    #20 0x7f13532c487c in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) /dom/base/nsGlobalWindow.cpp:10473:0
    #21 0x7f13532a8f72 in nsGlobalWindow::RunTimeout(nsTimeout*) /dom/base/nsGlobalWindow.cpp:10717:0
    #22 0x7f13532c3abc in nsGlobalWindow::TimerCallback(nsITimer*, void*) /dom/base/nsGlobalWindow.cpp:10964:0
    #23 0x7f1355c4bda0 in nsTimerImpl::Fire() /xpcom/threads/nsTimerImpl.cpp:546:0
    #24 0x7f1355c4c456 in nsTimerEvent::Run() /xpcom/threads/nsTimerImpl.cpp:630:0
    #25 0x7f1355c41a69 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:622:0
    #26 0x7f1355b685b1 in NS_ProcessNextEvent(nsIThread*, bool) /obj-firefox/xpcom/build/nsThreadUtils.cpp:238:0
    #27 0x7f13549a29e0 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:116:0
    #28 0x7f1355d66b53 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:220:0
    #29 0x7f135478915c in nsBaseAppShell::Run() /widget/xpwidgets/nsBaseAppShell.cpp:161:0
    #30 0x7f135416fc5e in nsAppStartup::Run() /toolkit/components/startup/nsAppStartup.cpp:269:0
    #31 0x7f135155ac00 in XREMain::XRE_mainRun() /toolkit/xre/nsAppRunner.cpp:3869:0
    #32 0x7f135155bb55 in XREMain::XRE_main(int, char**, nsXREAppData const*) /toolkit/xre/nsAppRunner.cpp:3937:0
    #33 0x7f135155ca8b in XRE_main /toolkit/xre/nsAppRunner.cpp:4139:0
    #34 0x459c8d in do_main(int, char**, nsIFile*) /browser/app/nsBrowserApp.cpp:275:0
    #35 0x7f1360aa9ea4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ea4)
    #36 0x45910c in ?? ??:0:0
0x6160001a7990 is located 16 bytes inside of 600-byte region [0x6160001a7980,0x6160001a7bd8)
freed by thread T0 here:
    #0 0x446015 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:0
    #1 0x7f1352121db8 in PresShell::Release() /layout/base/nsPresShell.cpp:712:0
previously allocated by thread T0 here:
    #0 0x446155 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74:0
    #1 0x7f135c0ae5c8 in moz_xmalloc /memory/mozalloc/mozalloc.cpp:54:0
    #2 0x7f13520aa8c3 in nsDocumentViewer::InitPresentationStuff(bool) /layout/base/nsDocumentViewer.cpp:661:0
    #3 0x7f13520aa1a8 in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, nsIntRect const&, bool, bool, bool) /layout/base/nsDocumentViewer.cpp:907:0
    #4 0x7f13520a92f0 in nsDocumentViewer::Init(nsIWidget*, nsIntRect const&) /layout/base/nsDocumentViewer.cpp:643:0
    #5 0x7f1353f84917 in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) /docshell/base/nsDocShell.cpp:6357:0
    #6 0x7f1353f96e8d in nsDocShell::CreateContentViewer(char const*, nsIRequest*, nsIStreamListener**) /docshell/base/nsDocShell.cpp:8133:0
    #7 0x7f1353f3af5e in nsDSURIContentListener::DoContent(char const*, bool, nsIRequest*, nsIStreamListener**, bool*) /docshell/base/nsDSURIContentListener.cpp:122:0
    #8 0x7f1353fde272 in nsDocumentOpenInfo::TryContentListener(nsIURIContentListener*, nsIChannel*) /uriloader/base/nsURILoader.cpp:680:0
    #9 0x7f1353fdbc1c in nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) /uriloader/base/nsURILoader.cpp:382:0
    #10 0x7f1353fdb38f in nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*) /uriloader/base/nsURILoader.cpp:258:0
    #11 0x7f13518113b2 in nsBaseChannel::OnStartRequest(nsIRequest*, nsISupports*) /netwerk/base/src/nsBaseChannel.cpp:720:0

Comment 2

5 years ago
There are at least two bugs, nsComputedDOMStyle::GetStyleContextForElement uses raw presshell pointer after flush and so does CanvasRenderingContext2D::SetFont


5 years ago
Keywords: sec-critical


5 years ago
Assignee: nobody → bugs

Comment 3

5 years ago
Created attachment 803085 [details] [diff] [review]

Actually this should be enough. We return early in canvas code if
this code doesn't end up succeeding.
Attachment #803085 - Flags: review?(bzbarsky)
Comment on attachment 803085 [details] [diff] [review]

Attachment #803085 - Flags: review?(bzbarsky) → review+

Comment 5

5 years ago
Comment on attachment 803085 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): at least 4 years old regression
User impact if declined: crashes
Testing completed (on m-c, etc.): NA 
Risk to taking this patch (and alternatives if risky): Should be super-safe 
String or IDL/UUID changes made by this patch: NA
Attachment #803085 - Flags: approval-mozilla-esr24?
Attachment #803085 - Flags: approval-mozilla-esr17?
Attachment #803085 - Flags: approval-mozilla-beta?
Attachment #803085 - Flags: approval-mozilla-b2g18?
Attachment #803085 - Flags: approval-mozilla-aurora?
Clearly the bug must not be that easy to stumble across if we haven't in 4 years of testing, including a year of fuzz testing on ASan builds, but the trivial patch is a big clue: adding a reference right before a flush. Not sure I'm comfortable exposing this 7 weeks before we can ship it, and I wouldn't count on getting a ride-along on a 24.0.1 because we really really don't want to have to do a 24.0.1
Keywords: csec-uaf
status-b2g18: --- → affected
status-firefox23: --- → wontfix
status-firefox24: --- → affected
status-firefox25: --- → affected
status-firefox26: --- → affected
status-firefox-esr17: --- → affected
status-firefox-esr24: --- → affected
tracking-b2g18: --- → ?
tracking-firefox24: --- → ?
tracking-firefox25: --- → +
tracking-firefox26: --- → +
tracking-firefox-esr17: --- → 25+
tracking-firefox-esr24: --- → +

Comment 7

5 years ago
I think it should be ok to land this very late during the next cycle.
tracking-b2g18: ? → 25+
tracking-firefox24: ? → +
tracking-firefox-esr24: + → 25+

Comment 8

5 years ago
It makes me uncomfortable that the line 'o131.mozTextStyle' causes the iframe's onresize event to fire, synchronously.  Is that a bug too?

Comment 9

5 years ago
(and why is the scroll() call needed to make it happen synchronously?)

Comment 10

5 years ago
FlushPendingNotifications explicitly fires pending resize events, very much on purpose. Though
perhaps these days we could fire it at the end of microtask.
How late in the cycle are we talking?  2nd to last week of beta 25?

Comment 12

5 years ago
That sounds good. The patch should be super safe.
Whiteboard: [land week of 10/14]
status-firefox24: affected → wontfix
status-firefox27: --- → affected
tracking-firefox24: + → ---
tracking-firefox27: --- → +
Whiteboard: [land week of 10/14] → [asan][land week of 10/14]
Keywords: checkin-needed
Whiteboard: [asan][land week of 10/14] → [asan][land week of 10/14][let's do this]
Attachment #803085 - Flags: approval-mozilla-esr24?
Attachment #803085 - Flags: approval-mozilla-esr24+
Attachment #803085 - Flags: approval-mozilla-esr17?
Attachment #803085 - Flags: approval-mozilla-esr17+
Attachment #803085 - Flags: approval-mozilla-beta?
Attachment #803085 - Flags: approval-mozilla-beta+
Attachment #803085 - Flags: approval-mozilla-b2g18?
Attachment #803085 - Flags: approval-mozilla-b2g18+
Attachment #803085 - Flags: approval-mozilla-aurora?
Attachment #803085 - Flags: approval-mozilla-aurora+

Will land on the release branches today after it's green on inbound.

Comment 14

5 years ago

Argh, sorry, I didn't see that message about m-i, and I landed to m-c.
I hope I didn't ruin merging.
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [asan][land week of 10/14][let's do this] → [asan]
status-b2g18: affected → fixed
status-b2g-v1.1hd: --- → affected
status-b2g-v1.2: --- → fixed
status-firefox25: affected → fixed
status-firefox26: affected → fixed
status-firefox27: affected → fixed
status-firefox-esr17: affected → fixed
status-firefox-esr24: affected → fixed
Keywords: checkin-needed
Target Milestone: --- → mozilla27
Matt, can you please verify this is fixed across all patched branches?
Flags: needinfo?(mwobensmith)
Keywords: verifyme
Confirmed crash on pre-patch ASan build of 24esr.
Verified fix on ASan builds of 24esr, 25, 26 and 27, 2013-10-15.

I'm having build issues with 17esr at the moment, and we'll have to verify B2G later.
status-firefox25: fixed → verified
status-firefox26: fixed → verified
status-firefox27: fixed → verified
status-firefox-esr24: fixed → verified
Flags: needinfo?(mwobensmith)
Keywords: verifyme
Whiteboard: [asan] → [asan][adv-main25+][adv-esr1710+][adv-esr24-1+]
Alias: CVE-2013-5599
Verified 17esr, 2013-10-23.
status-firefox-esr17: fixed → verified
Group: core-security
You need to log in before you can comment on or make changes to this bug.