832899 - crash with __proto__ and Events (destructor of xul!mozilla::dom::battery::BatteryManager)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nils]

Attachment: testcase (crashes firefox)

The latest nightly crashes when loading the attached testcase. Crashes while trying to write to invalid memory.

Stack backtrace on windows:

xul!mozilla::dom::battery::BatteryManager::`scalar deleting destructor'+0x6:
7041b694 c746204c019170  mov     dword ptr [esi+20h],offset xul!nsDisplayItemGeometry::`vftable' (7091014c) ds:002b:7097fe34={xul![thunk]:mozilla::dom::network::Connection::QueryInterface`adjustor{28}' (6fd2fdc4)}
0:000:x86> cdb: Reading initial command 'kp 16;q'
ChildEBP RetAddr  
0058b9f0 6ff1e325 xul!mozilla::dom::battery::BatteryManager::`scalar deleting destructor'(void)+0x6
0058ba04 7055f1dc xul!mozilla::dom::EventTarget::DispatchEvent(class nsIDOMEvent * aEvent = 0x0a694d60, class mozilla::ErrorResult * aRv = 0x0058ba28)+0x17
0058ba34 6fcb2541 xul!mozilla::dom::EventTargetBinding::dispatchEvent(struct JSContext * cx = 0x084c8080, class JS::Handle<JSObject *> obj = class JS::Handle<JSObject *>, class mozilla::dom::EventTarget * self = 0x08417c5c, unsigned int argc = 1, class JS::Value * vp = 0x04e100b8)+0xd0
*** WARNING: Unable to verify checksum for C:\newgenff\firefox\mozjs.dll
0058ba6c 733bc2df xul!mozilla::dom::EventTargetBinding::genericMethod(struct JSContext * cx = 0x084c8080, unsigned int argc = 1, class JS::Value * vp = 0x08417c5c)+0x6b
0058bb28 733c1d43 mozjs!js::InvokeKernel(struct JSContext * cx = 0x084c8080, class JS::CallArgs args = class JS::CallArgs, js::MaybeConstruct construct = NO_CONSTRUCT (0n0))+0xcf
0058c540 733bbdb9 mozjs!js::Interpret(struct JSContext * cx = 0x7097fe14, class js::StackFrame * entryFrame = 0x04e10028, js::InterpMode interpMode = JSINTERP_NORMAL (0n0))+0x7b3
0058c58c 733af318 mozjs!js::RunScript(struct JSContext * cx = 0x084c8080, class JS::Handle<JSScript *> script = class JS::Handle<JSScript *>, class js::StackFrame * fp = 0x04e10028)+0x99
0058c5d8 7342eb04 mozjs!js::ExecuteKernel(struct JSContext * cx = 0x084c8080, class JS::Handle<JSScript *> script = class JS::Handle<JSScript *>, struct JSObject * scopeChain = 0x057a0040, class JS::Value * thisv = 0x0058c600, js::ExecuteType type = EXECUTE_GLOBAL (0n1), class js::StackFrame * evalInFrame = 0x00000000, class JS::Value * result = 0x0058c758)+0x128
0058c608 7342ebed mozjs!js::Execute(struct JSContext * cx = 0x7097fe14, class JS::Handle<JSScript *> script = class JS::Handle<JSScript *>, struct JSObject * scopeChainArg = 0x08417c5c, class JS::Value * rval = 0x0058c758)+0x84
0058c66c 6fbef997 mozjs!JS::Evaluate(struct JSContext * cx = 0x084c8080, class JS::Handle<JSObject *> obj = class JS::Handle<JSObject *>, struct JS::CompileOptions options = struct JS::CompileOptions, wchar_t * chars = 0x0a654600 "window.start_dyniframe6()", unsigned int length = 0x19, class JS::Value * rval = 0x0058c758)+0xcd
0058c704 6fbfbd11 xul!nsJSContext::EvaluateString(class nsAString_internal * aScript = 0x0058c74c, struct JSObject * aScopeObject = 0x057a0040, struct JS::CompileOptions * aOptions = 0x0058c72c, bool aCoerceToString = false, class JS::Value * aRetValue = 0x0058c758)+0x267
0058c780 6fbfb938 xul!nsGlobalWindow::RunTimeoutHandler(struct nsTimeout * aTimeout = 0x0140a2e0, class nsIScriptContext * aScx = 0x061846a0)+0x1d1
0058c818 6fc01b4f xul!nsGlobalWindow::RunTimeout(struct nsTimeout * aTimeout = 0x0840a2e0)+0x238
0058c830 6fbbce44 xul!nsGlobalWindow::TimerCallback(class nsITimer * aTimer = 0x0a69f6a0, void * aClosure = 0x0840a2e0)+0x1b
0058c870 6fbbccfe xul!nsTimerImpl::Fire(void)+0x124
0058c87c 6fbbb474 xul!nsTimerEvent::Run(void)+0x1e
0058c8e8 6fcb880a xul!nsThread::ProcessNextEvent(bool mayWait = false, bool * result = 0x0058c91a)+0x1b4
0058c91c 6fcbf4cc xul!mozilla::ipc::MessagePump::Run(class base::MessagePump::Delegate * aDelegate = 0x00f300e0)+0xca
0058c954 6fcbf474 xul!MessageLoop::RunHandler(void)+0x21
0058c970 6fca227c xul!MessageLoop::Run(void)+0x15
0058c980 6fcbf3f4 xul!nsBaseAppShell::Run(void)+0x36
0058e8d4 6fcf2054 xul!nsAppShell::Run(void)+0x4e

Comment 1

[Nils]

Stack backtrace on Linux. Crashes while trying to execute unmapped memory:

#0  0xffffffffffffffc0 in ?? ()
#1  0x00007f7cab5693e2 in mozilla::dom::EventTargetBinding::dispatchEvent(JSContext*, JS::Handle<JSObject*>, mozilla::dom::EventTarget*, unsigned int, JS::Value*) () from /home/fred/nightly/firefox/libxul.so
#2  0x00007f7cab568fb3 in mozilla::dom::EventTargetBinding::genericMethod(JSContext*, unsigned int, JS::Value*) () from /home/fred/nightly/firefox/libxul.so
#3  0x00007f7cac331391 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) () from /home/fred/nightly/firefox/libxul.so
#4  0x00007f7cac3278e5 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) () from /home/fred/nightly/firefox/libxul.so
#5  0x00007f7cac330f51 in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) () from /home/fred/nightly/firefox/libxul.so
#6  0x00007f7cac331128 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) ()
   from /home/fred/nightly/firefox/libxul.so
#7  0x00007f7cac3342ff in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) () from /home/fred/nightly/firefox/libxul.so
#8  0x00007f7cac2dece1 in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) ()
   from /home/fred/nightly/firefox/libxul.so
#9  0x00007f7cabe3c961 in nsJSContext::EvaluateString(nsAString_internal const&,---Type <return> to continue, or q <return> to quit---
 JSObject&, JS::CompileOptions&, bool, JS::Value*) ()
   from /home/fred/nightly/firefox/libxul.so
#10 0x00007f7cabe4bf98 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) () from /home/fred/nightly/firefox/libxul.so
#11 0x00007f7cabe4c2ca in nsGlobalWindow::RunTimeout(nsTimeout*) ()
   from /home/fred/nightly/firefox/libxul.so
#12 0x00007f7cabe4c48e in nsGlobalWindow::TimerCallback(nsITimer*, void*) ()
   from /home/fred/nightly/firefox/libxul.so
#13 0x00007f7cac17f226 in nsTimerImpl::Fire() ()
   from /home/fred/nightly/firefox/libxul.so
#14 0x00007f7cac17f305 in nsTimerEvent::Run() ()
   from /home/fred/nightly/firefox/libxul.so
#15 0x00007f7cac17bd52 in nsThread::ProcessNextEvent(bool, bool*) ()
   from /home/fred/nightly/firefox/libxul.so
#16 0x00007f7cac15516c in NS_ProcessNextEvent_P(nsIThread*, bool) ()
   from /home/fred/nightly/firefox/libxul.so
#17 0x00007f7cac0e3c61 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) () from /home/fred/nightly/firefox/libxul.so
#18 0x00007f7cac19dbf3 in MessageLoop::Run() ()
   from /home/fred/nightly/firefox/libxul.so
#19 0x00007f7cab3f8162 in nsBaseAppShell::Run() ()

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure our canonical isupports matches what various consumers expect.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Nils, thank you for finding this!

Looks like this is probably a problem in every release since Firefox 12.  :(

The result of this bug is that we're casting to the concrete type incorrectly, the calling the wrong virtual function.

Yes, it's as bad as it sounds.  ;)

Comment 4

[Boris Zbarsky [:bzbarsky]]

Actually, looks like WebIDL for XHR only landed in Firefox 14.

And more importantly, the change to do castNative stuff for EventTarget only landed in 20, in bug 818263.

So that's the good news.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 705043 [details] [diff] [review]
Make sure our canonical isupports matches what various consumers expect.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?  Not
   particularly easily unless I include 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?  No.

Which older supported branches are affected by this flaw?  Just Aurora 20.

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?  This patch should apply to Aurora 20.

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 818263
User impact if declined: Likely-exploitable crashes.
Testing completed (on m-c, etc.): Passes on this testcase.
Risk to taking this patch (and alternatives if risky): This should be quite low
   risk.
String or UUID changes made by this patch: None.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure our canonical isupports matches what various consumers expect.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

http://mxr.mozilla.org/mozilla-central/source/dom/network/src/Connection.cpp#31
http://mxr.mozilla.org/mozilla-central/source/dom/network/src/MobileConnection.cpp#58
http://mxr.mozilla.org/mozilla-central/source/dom/cellbroadcast/src/CellBroadcast.cpp#55
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsScreen.cpp#97 
http://mxr.mozilla.org/mozilla-central/source/dom/sms/src/SmsManager.cpp#36
http://mxr.mozilla.org/mozilla-central/source/dom/sms/src/SmsRequest.cpp#55
http://mxr.mozilla.org/mozilla-central/source/dom/icc/src/IccManager.cpp#35

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 705055 [details] [diff] [review]
Make sure our canonical isupports matches what various consumers expect.

Need to update still few place :(

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure our canonical isupports matches what various consumers expect.

Comment 10

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/91a8133741df

I will probably land the test(s) on m-c once I've landed a patch on aurora.  Sound reasonable?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/91a8133741df

Comment 12

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/563d11ca4f52

Comment 13

[Daniel Veditz [:dveditz]]

(In reply to Boris Zbarsky (:bz) from comment #10)
> I will probably land the test(s) on m-c once I've landed a patch on aurora. 
> Sound reasonable?

Yes.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Hmm.  Should this stay closed, then, after I land the tests?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Test added: https://hg.mozilla.org/integration/mozilla-inbound/rev/ae3fa6ecf422

Comment 17

[Joe Drew (not getting mail)]

Merged to central: https://hg.mozilla.org/mozilla-central/rev/ae3fa6ecf422