Last Comment Bug 802902 - (CVE-2012-4217) Heap-use-after-free in nsViewManager::ProcessPendingUpdates
(CVE-2012-4217)
: Heap-use-after-free in nsViewManager::ProcessPendingUpdates
Status: RESOLVED FIXED
[asan][adv-track-main17+]
: crash, csectype-uaf, regression, sec-critical, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86_64 All
: -- critical (vote)
: mozilla19
Assigned To: Mats Palmgren (:mats)
:
:
Mentors:
Depends on:
Blocks: 598482
  Show dependency treegraph
 
Reported: 2012-10-17 17:53 PDT by Abhishek Arya
Modified: 2014-07-24 14:37 PDT (History)
9 users (show)
dveditz: sec‑bounty+
mats: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
fixed
+
fixed
+
fixed
unaffected


Attachments
Testcase (309 bytes, text/html)
2012-10-17 17:53 PDT, Abhishek Arya
no flags Details
stacks (27.59 KB, text/html)
2012-10-18 23:19 PDT, Mats Palmgren (:mats)
no flags Details
fix (2.06 KB, patch)
2012-10-18 23:28 PDT, Mats Palmgren (:mats)
roc: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
dveditz: sec‑approval+
Details | Diff | Splinter Review

Description Abhishek Arya 2012-10-17 17:53:02 PDT
Created attachment 672617 [details]
Testcase

Reproduces on trunk.

=================================================================
==12412== ERROR: AddressSanitizer heap-use-after-free on address 0x7fdd01d75fa8 at pc 0x7fdd18f484cc bp 0x7fffb2f66e10 sp 0x7fffb2f66e08
READ of size 8 at 0x7fdd01d75fa8 thread T0
    #0 0x7fdd18f484cb in nsViewManager::ProcessPendingUpdates() view/src/nsViewManager.cpp:1212
    #1 0x7fdd1ae3e36b in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:475
    #2 0x7fdd1ae3e9d6 in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:555
    #3 0x7fdd1ad6baf3 in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan/xpcom/build/nsThreadUtils.cpp:220
    #4 0x7fdd1a92b6b0 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
    #5 0x7fdd1aec3bdb in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:215
    #6 0x7fdd1a624ebd in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
    #7 0x7fdd17891a44 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3858
    #8 0x7fdd17892b1a in XRE_main toolkit/xre/nsAppRunner.cpp:3933
    #9 0x408d75 in do_main(int, char**) browser/app/nsBrowserApp.cpp:174
    #10 0x7fdd219e876c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
0x7fdd01d75fa8 is located 40 bytes inside of 64-byte region [0x7fdd01d75f80,0x7fdd01d75fc0)
freed by thread T0 here:
    #0 0x432e00 in free 
    #1 0x7fdd18f3ebac in nsViewManager::Release() view/src/nsViewManager.cpp:125
    #2 0x7fdd19eaf9d8 in nsDocShell::Destroy() docshell/base/nsDocShell.cpp:4897
    #3 0x7fdd19eaff5c in non-virtual thunk to nsDocShell::Destroy() :0
    #4 0x7fdd1a06e13d in nsWebShellWindow::Destroy() xpfe/appshell/src/nsWebShellWindow.cpp:750
    #5 0x7fdd18ff4fec in nsGlobalWindow::ReallyCloseWindow() dom/base/nsGlobalWindow.cpp:6734
    #6 0x7fdd18ff433f in nsGlobalWindow::FinalClose() dom/base/nsGlobalWindow.cpp:6681
    #7 0x7fdd18ff3ca2 in nsGlobalWindow::Close() dom/base/nsGlobalWindow.cpp:6624
    #8 0x7fdd18ff3811 in nsGlobalWindow::Close() dom/base/nsGlobalWindow.cpp:6559
    #9 0x7fdd1ae734a9 in NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
    #10 0x7fdd19bee7c9 in CallMethodHelper::Invoke() js/xpconnect/src/XPCWrappedNative.cpp:3108
    #11 0x7fdd19c02dc8 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1488
    #12 0x7fdd1c0e3148 in JSFunction::native() const js/src/jscntxtinlines.h:364
    #13 0x7fdd1c0e40b7 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:109
    #14 0x7fdd1c1c8527 in js::IndirectProxyHandler::call(JSContext*, JSObject*, unsigned int, JS::Value*) js/src/jsproxy.cpp:450
    #15 0x7fdd1c32730d in js::DirectWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) js/src/jswrapper.cpp:390
    #16 0x7fdd1c327639 in non-virtual thunk to js::CrossCompartmentWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) 
    #17 0x7fdd1c0e3289 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:364
    #18 0x7fdd1c0d30d9 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2370
    #19 0x7fdd1c0bdf33 in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) js/src/jsinterp.cpp:324
    #20 0x7fdd1c0e3038 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:378
    #21 0x7fdd1c040be0 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:109
    #22 0x7fdd1c0e3148 in JSFunction::native() const js/src/jscntxtinlines.h:364
    #23 0x7fdd1c0d30d9 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2370
    #24 0x7fdd1c0bdf33 in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) js/src/jsinterp.cpp:324
    #25 0x7fdd1c0e3038 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:378
    #26 0x7fdd1c0e40b7 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:109
    #27 0x7fdd1c1c8527 in js::IndirectProxyHandler::call(JSContext*, JSObject*, unsigned int, JS::Value*) js/src/jsproxy.cpp:450
    #28 0x7fdd1c32730d in js::DirectWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) js/src/jswrapper.cpp:390
    #29 0x7fdd1c327639 in non-virtual thunk to js::CrossCompartmentWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) 
previously allocated by thread T0 here:
    #0 0x432ec0 in __interceptor_malloc 
    #1 0x7fdd1ee8d228 in moz_xmalloc memory/mozalloc/mozalloc.cpp:57
Shadow byte and word:
  0x1ffba03aebf5: fd
  0x1ffba03aebf0: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ffba03aebd0: 00 00 00 01 fb fb fb fb
  0x1ffba03aebd8: fb fb fb fb fb fb fb fb
  0x1ffba03aebe0: fa fa fa fa fa fa fa fa
  0x1ffba03aebe8: fa fa fa fa fa fa fa fa
=>0x1ffba03aebf0: fd fd fd fd fd fd fd fd
  0x1ffba03aebf8: fd fd fd fd fd fd fd fd
  0x1ffba03aec00: fa fa fa fa fa fa fa fa
  0x1ffba03aec08: fa fa fa fa fa fa fa fa
  0x1ffba03aec10: fd fd fd fd fd fd fd fd
Stats: 289M malloced (302M for red zones) by 407162 calls
Stats: 35M realloced by 20307 calls
Stats: 252M freed by 260279 calls
Stats: 121M really freed by 107074 calls
Stats: 500M (128096 full pages) mmaped in 125 calls
  mmaps   by size class: 8:262128; 9:32764; 10:12285; 11:12282; 12:3072; 13:2048; 14:1280; 15:256; 16:576; 17:1280; 18:288; 19:40; 20:32; 21:4;
  mallocs by size class: 8:326930; 9:43810; 10:12490; 11:13674; 12:3072; 13:2374; 14:1671; 15:339; 16:815; 17:1616; 18:297; 19:40; 20:31; 21:3;
  frees   by size class: 8:201346; 9:31687; 10:7929; 11:10578; 12:2184; 13:2179; 14:1470; 15:284; 16:725; 17:1600; 18:228; 19:39; 20:27; 21:3;
  rfrees  by size class: 8:79435; 9:13591; 10:2500; 11:7030; 12:870; 13:866; 14:1033; 15:156; 16:460; 17:1116; 18:11; 19:5; 20:1;
Stats: malloc large: 1987 small slow: 2137
Comment 1 Olli Pettay [:smaug] 2012-10-17 18:36:20 PDT
Managed to get a crash.

Um, few bugs here.
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsRefreshDriver.cpp?rev=3de1ec1e38cf&mark=431-431#425
doesn't ensure the viewmanager stays alive.

nsViewManager::ProcessPendingUpdates() is all scary.
CallWillPaintOnObservers(true); flushes, so things which aren't kept explicitly alive
can be dead. And in this case for some reason mRootView is deleted
http://mxr.mozilla.org/mozilla-central/source/view/src/nsViewManager.cpp?rev=eaccb5bb50c0&mark=1213-1213#1199
(Could be that the viewmanager itself is dead at that point)
Comment 2 Olli Pettay [:smaug] 2012-10-17 18:59:59 PDT
Oh, right, I got the crash when shutting down the browser. So it is the top level
presshell and its viewmanager which are dead.
Comment 3 David Bolter [:davidb] 2012-10-18 13:20:06 PDT
Thanks for the quick triage! Assigning to Mats, please reassign as necessary. Any help marking status tracking (FF versions affected) appreciated.
Comment 4 Mats Palmgren (:mats) 2012-10-18 23:19:25 PDT
Created attachment 673106 [details]
stacks

1st stack:
Destroy of the shell involved in RefreshDriver::Notify in stack frame #61

2nd stack:
~nsViewManager of the view manager involved in same RefreshDriver::Notify

3rd stack:
ASAN error when touching mRootView of that dead view manager
Comment 5 Mats Palmgren (:mats) 2012-10-18 23:23:42 PDT
I strongly suspect it's a regression from bug 598482 which added this code
in RefreshDriver:  http://hg.mozilla.org/mozilla-central/rev/2d5e31af7e3e
If so, it affects all branches except esr10.
Comment 6 Mats Palmgren (:mats) 2012-10-18 23:28:16 PDT
Created attachment 673110 [details] [diff] [review]
fix

The fix is the strong ref in RefreshDriver.  I added strong refs for
the WillPaintWindow and DidPaintWindow calls as well, just in case.
Comment 7 Mats Palmgren (:mats) 2012-10-19 08:24:07 PDT
Comment on attachment 673110 [details] [diff] [review]
fix

The fix is very low risk. 
We should take this on all branches where it applies.
Comment 8 Daniel Veditz [:dveditz] 2012-10-19 22:23:42 PDT
Comment on attachment 673110 [details] [diff] [review]
fix

sec-approval+
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-10-22 19:09:42 PDT
https://hg.mozilla.org/mozilla-central/rev/e0f277dac81a
Comment 12 Daniel Veditz [:dveditz] 2012-11-19 15:26:19 PST
This bug qualifies for a security bug bounty.
Comment 14 Mats Palmgren (:mats) 2013-05-13 07:34:06 PDT
Crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe7d6a66997c
Comment 15 Ryan VanderMeulen [:RyanVM] 2013-05-13 13:45:05 PDT
https://hg.mozilla.org/mozilla-central/rev/fe7d6a66997c
Comment 16 Tracy Walker [:tracy] 2014-01-10 10:39:42 PST
mass remove verifyme requests greater than 4 months old

Note You need to log in before you can comment on or make changes to this bug.