Closed
Bug 474456
Opened 16 years ago
Closed 16 years ago
[1.9.0]Mozilla Firefox XUL Linked Clones Double Free Vulnerability (ZDI-CAN-423)
Categories
(Core :: XPConnect, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: dveditz, Assigned: peterv)
References
Details
(4 keywords, Whiteboard: [sg:critical] fixed in 1.9.1+ by bug 457022, 1.9.0-only)
Attachments
(1 file, 1 obsolete file)
1.43 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
jst
:
approval1.9.1+
dveditz
:
approval1.9.0.7+
|
Details | Diff | Splinter Review |
TippingPoint has identified a vulnerability affecting the following products: Mozilla Firefox 3.0.x -- VULNERABILITY DETAILS ----------------------------------------------- This vulnerability allows remote attackers to execute arbitrary code on vulnerable installations of Mozilla Firefox. User interaction is required to exploit this vulnerability in that the target must visit a malicious page. The specific flaw exists during the browsers garbage collection process. When multiple DOM elements are cloned and linked to one another and the browser is reloaded a memory corruption occurs resulting in a double free. This can be leveraged to execute arbitrary code under the context of the current user. The following basic JavaScript will reproduce the issue: body = document.getElementById('mainbody'); noti = document.createElementNS(xulns,'notification'); body.appendChild(noti); elem = document.createElement("div"); statusbar = document.createElementNS(xulns,'statusbarpanel'); clone = statusbar.cloneNode(1); elem.appendChild(clone); for(var x=0;x<10;x++) hs(); location.reload() The body element must be connected to the DOM. After a few reloads of the website the browser will crash, in most cases during garbage collection, because it tries to free an object twice. The elem object can be any element (HTML, SVG, XUL, etc.) The notification object has to be created and appended to the body element. Just cloning the statusbarpanel object and appending it to any element, will not trigger the vulnerability. -- CREDIT -------------------------------------------------------------- This vulnerability was discovered by: * Anonymous
Reporter | ||
Comment 1•16 years ago
|
||
Seeking more detail and PoC from ZDI. E.g. what does the hs() function do? Seems like it might be an important part of this.
Whiteboard: [sg:needinfo]
Comment 2•16 years ago
|
||
Also is this a XUL document or an HTML document?
Reporter | ||
Comment 3•16 years ago
|
||
Reporter | ||
Comment 4•16 years ago
|
||
Testcase crashes on Mac 1.9.0.7pre during GC. 0 libxpconnect.dylib 0x00d28eca XPCJSRuntime::GCCallback(JSContext*, JSGCStatus) + 1716 (xpcjsruntime.cpp:818) 1 libgklayout.dylib 0x131dda74 DOMGCCallback(JSContext*, JSGCStatus) + 48 (nsJSEnvironment.cpp:3517) 2 libmozjs.dylib 0x00232899 js_GC + 4378 (jsgc.c:3536) 3 libmozjs.dylib 0x0022e1ff js_NewGCThing + 673 (jsgc.c:1765) 4 libmozjs.dylib 0x002af224 js_NewString + 72 (jsstr.c:2467) 5 libmozjs.dylib 0x002a76ec js_ConcatStrings + 986 (jsstr.c:167) 6 libmozjs.dylib 0x00239fb5 js_Interpret + 27544 (jsinterp.c:3684) 7 libmozjs.dylib 0x0024fde3 js_Execute + 865 (jsinterp.c:1544) 8 libmozjs.dylib 0x001ef8b0 JS_EvaluateUCScriptForPrincipals + 221 (jsapi.c:4999) 9 libgklayout.dylib 0x131e11d2 nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*) + 916 (nsJSEnvironment.cpp:1530) 10 libgklayout.dylib 0x13210f1f nsGlobalWindow::RunTimeout(nsTimeout*) + 1423 (nsGlobalWindow.cpp:7875) 11 libgklayout.dylib 0x132115d8 nsGlobalWindow::TimerCallback(nsITimer*, void*) + 54 (nsGlobalWindow.cpp:8228) 12 libxpcom_core.dylib 0x003b1d60 nsTimerImpl::Fire() + 868 (nsTimerImpl.cpp:401) 13 libxpcom_core.dylib 0x003b1f8e nsTimerEvent::Run() + 192 (nsTimerImpl.cpp:492) 14 libxpcom_core.dylib 0x003ab77a nsThread::ProcessNextEvent(int, int*) + 676 (nsThread.cpp:511) 15 libxpcom_core.dylib 0x003365d4 NS_ProcessPendingEvents_P(nsIThread*, unsigned int) + 146 (nsThreadUtils.cpp:180) 16 libwidget_mac.dylib 0x11b44d0b nsBaseAppShell::NativeEventCallback() + 181 (nsBaseAppShell.cpp:122) 17 libwidget_mac.dylib 0x11b003d6 nsAppShell::ProcessGeckoEvents(void*) + 518 (nsAppShell.mm:303) 18 com.apple.CoreFoundation 0x945e063f CFRunLoopRunSpecific + 3215 19 com.apple.CoreFoundation 0x945e0cd8 CFRunLoopRunInMode + 88 20 com.apple.HIToolbox 0x948382c0 RunCurrentEventLoopInMode + 283 21 com.apple.HIToolbox 0x948380d9 ReceiveNextEventCommon + 374 22 com.apple.HIToolbox 0x94837f4d BlockUntilNextEventMatchingListInMode + 106 23 com.apple.AppKit 0x94e7cd7d _DPSNextEvent + 657 24 com.apple.AppKit 0x94e7c630 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128 25 com.apple.AppKit 0x94e7566b -[NSApplication run] + 795 26 libwidget_mac.dylib 0x11afed8c nsAppShell::Run() + 288 (nsAppShell.mm:591) 27 libtoolkitcomps.dylib 0x127566cc nsAppStartup::Run() + 148 (nsAppStartup.cpp:181) 28 XUL 0x000f7a11 XRE_main + 13853 (nsAppRunner.cpp:3193) 29 org.mozilla.firefox 0x000026d3 main + 709 (nsBrowserApp.cpp:158) 30 org.mozilla.firefox 0x00001d00 _start + 210 31 org.mozilla.firefox 0x00001c2d start + 41
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.9.0.x+
Flags: blocking1.9.1?
Flags: blocking1.9.0.7?
Whiteboard: [sg:needinfo] → [sg:critical]
Updated•16 years ago
|
Reporter | ||
Comment 5•16 years ago
|
||
This may be fixed in 1.9.1, my Mac hasn't crashed. I'll try the testcase in a windows VM next but it may have been fixed by one of the JS bugs Jesse already smoked out.
Comment 6•16 years ago
|
||
This seems to have been fixed between 2008-11-13 and 2008-11-14: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-11-13+05%3A00%3A00&enddate=2008-11-14+06%3A00%3A00 Fixed somehow by bug 425153?
![]() |
||
Comment 7•16 years ago
|
||
That doesn't seem very likely.... Bug 457022 is in the range too, but also doesn't seem that likely. Is someone willing to try bisecting?
This is surely JS, XPConnect or content. Moving to XPConnect.
Component: XUL → XPConnect
QA Contact: xptoolkit.widgets → xpconnect
Comment 9•16 years ago
|
||
Sounds like this is not an issue in 1.9.1, so not blocking.
Flags: blocking1.9.1? → blocking1.9.1-
Reporter | ||
Updated•16 years ago
|
Summary: Mozilla Firefox XUL Linked Clones Double Free Vulnerability (ZDI-CAN-423) → [1.9.0]Mozilla Firefox XUL Linked Clones Double Free Vulnerability (ZDI-CAN-423)
Whiteboard: [sg:critical] → [sg:critical] fixed in 1.9.1
Reporter | ||
Comment 10•16 years ago
|
||
peterv: looking at that fix range you're the most likely fixer, could you investigate whether your checkins fixed this 1.9.0-blocking externally-reported security bug? Thanks!
Assignee: nobody → peterv
Flags: blocking1.9.0.7? → blocking1.9.0.7+
Comment 11•16 years ago
|
||
Peterv: Any luck on this?
Assignee | ||
Comment 12•16 years ago
|
||
I bisected and this is definitely fixed by 457022, but we can't take that on branch. Trying to figure out what's going on.
Assignee | ||
Comment 13•16 years ago
|
||
This was a hard one to debug, but it's a simple problem. An element is destroyed and its destructor calls UnbindFromTree on its children, one of which is a XUL element. UnbindFromTree unhooks any XBL bindings and we end up in nsXBLBinding::ChangeDocument. ChangeDocument tries to wrap the XUL element but for some reason there's no existing wrapper anymore (we probably unlinked the document from cycle collection?) and we create a new wrapper. The PreCreate hook for the XUL element uses the parent node as the parent for the wrapper, the parent node is the element that is being destroyed and so we wrap that dying object. At some point the wrapper tries to access its native object, but it is already dead. The fix is to null out the parent before unhooking XBL bindings. I made nulling the parent the first thing in UnbindFromTree, none of the things we did before it really need the parent. The reason why this is fixed on trunk is probably because we store the wrapper in the element and not in a hash in the ownerDocument, so the wrapper for XBL-bound nodes stays alive even when the ownerDocument is unlinked or dies (and the parent is kept alive too). I think we should still take this fix on trunk too, it looks safer.
Attachment #360558 -
Flags: superreview?(bzbarsky)
Attachment #360558 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 14•16 years ago
|
||
Sorry, wrong patch. We must not unset PARENT_BIT_INDOCUMENT before calling GetCurrentDoc.
Attachment #360558 -
Attachment is obsolete: true
Attachment #360562 -
Flags: superreview?(bzbarsky)
Attachment #360562 -
Flags: review?(bzbarsky)
Attachment #360558 -
Flags: superreview?(bzbarsky)
Attachment #360558 -
Flags: review?(bzbarsky)
![]() |
||
Updated•16 years ago
|
Attachment #360562 -
Flags: superreview?(bzbarsky)
Attachment #360562 -
Flags: superreview+
Attachment #360562 -
Flags: review?(bzbarsky)
Attachment #360562 -
Flags: review+
Reporter | ||
Updated•16 years ago
|
Attachment #360562 -
Flags: approval1.9.1?
Attachment #360562 -
Flags: approval1.9.0.7+
Reporter | ||
Comment 15•16 years ago
|
||
Comment on attachment 360562 [details] [diff] [review] v1 Approved for 1.9.0.7, a=dveditz for release-drivers.
Assignee | ||
Comment 16•16 years ago
|
||
Checked in to trunk and 1.9.0.x branch. Haven't checked in the testcase.
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: fixed1.9.0.7
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Reporter | ||
Comment 17•16 years ago
|
||
Comment on attachment 360562 [details] [diff] [review] v1 This is already in 1.9.1
Attachment #360562 -
Flags: approval1.9.1?
Reporter | ||
Comment 18•16 years ago
|
||
peterv: Although the fixed code in UnbindFromTree looks similar on the 1.8 branch, this exploit doesn't crash the browser. I'm sure notifications have changed a lot between the two branches. I'm assuming we don't need this fix on the 1.8 branch, but please renominate for blocking1.8.1.next if you think it would be safer if we took it.
Depends on: 457022
Flags: blocking1.8.1.next-
Whiteboard: [sg:critical] fixed in 1.9.1 → [sg:critical] fixed in 1.9.1+ by bug 457022, 1.9.0-only
Assignee | ||
Updated•16 years ago
|
Attachment #360562 -
Flags: approval1.9.1?
Assignee | ||
Comment 19•16 years ago
|
||
Comment on attachment 360562 [details] [diff] [review] v1 This hasn't landed on 1.9.1. Like on trunk the crash is already fixed by bug 457022 but I still think it'd be a good idea to take this patch on branch because it's safer and low-risk.
Assignee | ||
Comment 20•16 years ago
|
||
(In reply to comment #18) > I'm assuming we don't need this fix on > the 1.8 branch, but please renominate for blocking1.8.1.next if you think it > would be safer if we took it. I think we probably don't need it on 1.8.1 because we don't have cycle collection and so the wrappers don't get unlinked.
Comment 21•16 years ago
|
||
Verified the crashes using the POC testcase with 1.9.0.6 and the fix in 1.9.0.7 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009021304 GranParadiso/3.0.7pre. This is fixed now.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.0.7 → verified1.9.0.7
Comment 22•16 years ago
|
||
Comment on attachment 360562 [details] [diff] [review] v1 I think we should take this on the branch to be consistent with the trunk, even if it's not strictly needed there.
Attachment #360562 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Updated•16 years ago
|
Keywords: fixed1.9.1
Updated•16 years ago
|
Flags: wanted1.8.1.x-
Updated•16 years ago
|
Group: core-security
Updated•16 years ago
|
Flags: wanted1.8.0.x-
Flags: blocking1.8.0.next-
Comment 23•16 years ago
|
||
Um, I'm guessing my above comment was either meant for a different bug, or I was really confused when I made the above comment...
Comment 24•16 years ago
|
||
peterv, do you know if there's a simple testcase for this bug that can be used as a crashtest?
You need to log in
before you can comment on or make changes to this bug.
Description
•