817808 - Crash in NodeListWrapper::getNative dombindings_gen.cpp line#628

Status: RESOLVED WORKSFORME

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

Description

[Cody Crews]

Attachment: crashff.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121128204232

Steps to reproduce:

Created a document with an iframe and object both with the url of about:blank, then created a function binding iframe.contentDocument.childNodes.item to object.contentDocument.childNodes and called the function.


Actual results:

Firefox crashes when xul.dll references a null pointer and tries to write to its location.


Expected results:

Calling the created function should have returned a childnode from object.contentDocument.

Comment 1

[Cody Crews]

Not sure if this is exploitable, but was told I should mark it security sensitive just in case.  The crash actually happens after other problems as a bailout in the line number listed.  I expect the line number could be different as this is from the 17.0 source but it has been tested on 17.0.1.  Starting tomorrow I probably wont be around for a while so I thought I should put this out there before then.  I wish I had more time to actually bring something helpful to the table with these bugs.

Comment 2

[Al Billings [:abillings - ex-MoCo]]

Confirmed. I just crashed with shipped Firefox in a standard profile on OS X 10.8 with this: bp-f45d40cf-f5b6-4b75-a994-2cc972121203

Comment 3

[Daniel Veditz [:dveditz]]

As long as it's always null this should be a relatively safe crash. It's possible it _will_ always be either null or a valid pointer because of the way our nsCOMPtrs work, but that's just a general rule and may not apply to this case once it's investigated.

Comment 4

[Daniel Veditz [:dveditz]]

Matt: please run this test in an ASan build and attach the log here. That will give us more information about what's going on.

Comment 5

[Matt Wobensmith [:mwobensmith][:matt:]]

Does not crash latest nightly or Aurora, ASan or normal builds.

However, I created an ASan release build 17.0.1 so that we could have a symbolized stack trace - below.


Assertion failure: objIsList(obj), at /Users/mwobensmith/current_release/js/xpconnect/src/dombindings.cpp:178
ASAN:SIGSEGV
=================================================================
==63058== ERROR: AddressSanitizer crashed on unknown address 0x000000000000 (pc 0x000107f7b588 sp 0x7fff5fbf1980 bp 0x7fff5fbf1a30 T0)
AddressSanitizer can not provide additional info.
    #0 0x107f7b587 in mozilla::dom::oldproxybindings::ListBase<mozilla::dom::oldproxybindings::ListClass<nsINodeList, mozilla::dom::oldproxybindings::Ops<mozilla::dom::oldproxybindings::Getter<nsIContent*>, mozilla::dom::oldproxybindings::NoOp>, mozilla::dom::oldproxybindings::Ops<mozilla::dom::oldproxybindings::NoOp, mozilla::dom::oldproxybindings::NoOp> > >::getListObject dombindings.cpp:178
    #1 0x107f593c7 in mozilla::dom::oldproxybindings::NodeList_Item .dombindings_gen.cpp:657
    #2 0x10a7ea9da in js::CallJSNative, JS::CallArgs const&) jscntxtinlines.h:372
    #3 0x10a7e2913 in js::InvokeKernel jsinterp.cpp:345
    #4 0x10a6d96ec in js::CallOrConstructBoundFunction jsinterp.h:119
    #5 0x10a7ea9da in js::CallJSNative, JS::CallArgs const&) jscntxtinlines.h:372
    #6 0x10a7e2913 in js::InvokeKernel jsinterp.cpp:345
    #7 0x10a7e4200 in js::Invoke jsinterp.h:119
    #8 0x10a8ed799 in js::IndirectProxyHandler::call jsproxy.cpp:483
    #9 0x10aa776c2 in js::CrossCompartmentWrapper::call jswrapper.cpp:404
    #10 0x10aa7788c in non-virtual thunk to js::CrossCompartmentWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) (in XUL) + 12
    #11 0x10a8fda72 in js::Proxy::call jsproxy.cpp:1332
    #12 0x10a903a86 in proxy_Call jsproxy.cpp:1888
    #13 0x10a7ea9da in js::CallJSNative, JS::CallArgs const&) jscntxtinlines.h:372
    #14 0x10a7e2913 in js::InvokeKernel jsinterp.cpp:345
    #15 0x10a7c22a7 in js::Interpret jsinterp.cpp:2414
    #16 0x10acf9372 in js::mjit::EnterMethodJIT MethodJIT.cpp:1043
    #17 0x10acf9e72 in CheckStackAndEnterMethodJIT MethodJIT.cpp:1074
    #18 0x10a78d4f7 in js::RunScript jsinterp.cpp:306
    #19 0x10a7e282a in js::InvokeKernel jsinterp.cpp:363
    #20 0x10a7e4200 in js::Invoke jsinterp.h:119
    #21 0x10a5e327e in JS_CallFunctionValue jsapi.cpp:5850
    #22 0x106cad3ab in nsJSContext::CallEventHandler nsJSEnvironment.cpp:1917
    #23 0x106f98a2c in nsJSEventListener::HandleEvent nsJSEventListener.cpp:188
    #24 0x1066a014e in nsEventListenerManager::HandleEventInternal nsEventListenerManager.cpp:800
    #25 0x10672ce87 in nsEventTargetChainItem::HandleEvent nsEventListenerManager.h:142
    #26 0x1067261a8 in nsEventTargetChainItem::HandleEventTargetChain nsEventDispatcher.cpp:314
    #27 0x10672b201 in nsEventDispatcher::Dispatch nsEventDispatcher.cpp:635
    #28 0x1059b8d61 in PresShell::HandleEventInternal nsPresShell.cpp:6444
    #29 0x1059ba46b in PresShell::HandleEventWithTarget nsPresShell.cpp:6228
    #30 0x1066d8db6 in nsEventStateManager::CheckForAndDispatchClick nsEventStateManager.cpp:4438
    #31 0x1066d0cca in nsEventStateManager::PostHandleEvent nsEventStateManager.cpp:3226
    #32 0x1059b8ebf in PresShell::HandleEventInternal nsPresShell.cpp:6469
    #33 0x1059b68a3 in PresShell::HandlePositionedEvent nsPresShell.cpp:6213
    #34 0x1059b59e2 in PresShell::HandleEvent nsPresShell.cpp:6010
    #35 0x106c5de67 in nsViewManager::DispatchEvent nsViewManager.cpp:772
    #36 0x106c5563c in nsView::HandleEvent nsView.cpp:1062
    #37 0x108a17a4a in nsChildView::DispatchEvent nsChildView.mm:1485
    #38 0x108a17e12 in nsChildView::DispatchWindowEvent nsChildView.mm:1493
    #39 0x108a287d9 in -[ChildView mouseUp:] nsChildView.mm:3329
    #40 0x7fff8d8f66d5 in -[NSWindow sendEvent:] (in AppKit) + 7052
    #41 0x108a0cc2d in -[ToolbarWindow sendEvent:] nsCocoaWindow.mm:2709
    #42 0x7fff8d8f2743 in -[NSApplication sendEvent:] (in AppKit) + 5760
    #43 0x1089f109a in -[GeckoNSApplication sendEvent:] nsAppShell.mm:157
    #44 0x7fff8d8082f9 in -[NSApplication run] (in AppKit) + 635
    #45 0x1089f3f19 in nsAppShell::Run nsAppShell.mm:756
    #46 0x10832b690 in nsAppStartup::Run nsAppStartup.cpp:273
    #47 0x1050c2a02 in XREMain::XRE_mainRun nsAppRunner.cpp:3812
    #48 0x1050c31ad in XREMain::XRE_main nsAppRunner.cpp:3889
    #49 0x1050c3b7a in XRE_main nsAppRunner.cpp:3965
    #50 0x100002c34 in main nsBrowserApp.cpp:174
    #51 0x1000015a3 in start (in firefox-bin) + 51
    #52 0x0 in 0x0000000100000000 (in firefox-bin)
Stats: 399M malloced (334M for red zones) by 500038 calls
Stats: 61M realloced by 19519 calls
Stats: 366M freed by 357893 calls
Stats: 206M really freed by 211318 calls
Stats: 576M (147556 full pages) mmaped in 135 calls
  mmaps   by size class: 8:245745; 9:40955; 10:24570; 11:18423; 12:4096; 13:2560; 14:1536; 15:1280; 16:448; 17:1280; 18:192; 19:40; 20:24; 21:4; 24:3;
  mallocs by size class: 8:357738; 9:63205; 10:39156; 11:24979; 12:4822; 13:3940; 14:2083; 15:1658; 16:610; 17:1536; 18:228; 19:47; 20:31; 21:3; 24:3;
  frees   by size class: 8:241279; 9:47676; 10:33967; 11:21768; 12:3728; 13:3674; 14:1874; 15:1600; 16:532; 17:1517; 18:201; 19:44; 20:27; 21:3; 24:3;
  rfrees  by size class: 8:131847; 9:34364; 10:21106; 11:16541; 12:1977; 13:1951; 14:1249; 15:560; 16:325; 17:1288; 18:51; 19:38; 20:21;
Stats: malloc large: 1902 small slow: 3095

Comment 6

[Boris Zbarsky [:bzbarsky]]

Hmm.  I'm surprised this is even triggered in 17; I thought we used the WebIDL bindings there.

In any case, sounds like ListBase<LC>::getListObject needs to null-check after unwrapping?  Peter?

Comment 7

[Cody Crews]

I'm not looking at the source atm but I remember seeing two conditional returns in the function mentioned above and neither gets triggered before the crash is.  My original thoughts on this were what happens to leave a null pointer in play, but you guys probably already have that figured out.

Comment 8

[Daniel Veditz [:dveditz]]

This does not look exploitable, unhiding. Since it doesn't occur in Aurora or nightly (19,20) do we want to just call this WORKSFORME and move on, or do you think we still need that null check?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Might want to add the null-check in 18, I guess, right?

Comment 11

[Bobby Holley (:bholley)]

Looks like this was in the old proxy bindings, so I doubt it's a problem anymore.