XPCNativeWrapper Crash [@ XPCNativeWrapper::GetNewOrUsed]

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: gfleischer+bugzilla, Assigned: mrbkap)

Tracking

({fixed1.9.1, verified1.9.0.12})

unspecified
fixed1.9.1, verified1.9.0.12
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.0.12 +
wanted1.9.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:dos] null deref)

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.16) Gecko/20080702 Firefox/2.0.0.16
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.16) Gecko/20080702 Firefox/2.0.0.16

Calls to XPCNativeWrapper crash when invoked on Components object in Sandbox.

Firefox 3 example:
<link rel="stylesheet" type="text/css" href="javascript:XPCNativeWrapper(Components);"></link>

Firefox 2 example:
<img alt="" src="javascript:XPCNativeWrapper(Components);"></img>

Appears to be a basic null-pointer dereference, but I occasionally noticed subsequent invalid memory accesses so there may be additional corruption occurring.  At a minimum, this would be an annoying denial of service.

This may also be Bug 352134.


Reproducible: Always




Tested with user-agents:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20080916020338 Minefield/3.1b1pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.18pre) Gecko/20080915 BonEcho/2.0.0.18pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.17) Gecko/20080827 Firefox/2.0.0.17
(Reporter)

Comment 1

10 years ago
Created attachment 338997 [details]
Generate crash using img tag

Test case to crash browser using img tag.
(Reporter)

Comment 2

10 years ago
Created attachment 338998 [details]
Generate crash using link tag

Test case to cause browser crash using link tag
Component: General → XPConnect
Product: Firefox → Core
QA Contact: general → xpconnect
Confirmed that at least the second attachment (crash via link tag) crashes for me on 3.0.1 / OS X 10.5
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [sg:critical]
It's also possible to hit this crash via PAC.
Created attachment 340640 [details] [diff] [review]
Proposed fix

As far as I can see, we have two options here:
- Null check when creating XPCNativeWrappers (taken here)
- Make the sandbox global object be a wrapped native

I think the second option is unnecessarily complicated, though I don't have terribly strong feelings on the matter. Boris, if you'd prefer, I'll implement the second option.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #340640 - Flags: superreview?(bzbarsky)
Attachment #340640 - Flags: review?(bzbarsky)
Comment on attachment 340640 [details] [diff] [review]
Proposed fix

Hmm....  I _think_ this should be fine...  Is there any danger in chrome using a sandbox and then touching the polluted global and being screwed?
Attachment #340640 - Flags: superreview?(bzbarsky)
Attachment #340640 - Flags: superreview+
Attachment #340640 - Flags: review?(bzbarsky)
Attachment #340640 - Flags: review+
(In reply to comment #6)
> Hmm....  I _think_ this should be fine...  Is there any danger in chrome using
> a sandbox and then touching the polluted global and being screwed?

Once an untrusted script has been evaluated in a sandbox, the only safe way to touch the sandbox object anyway is to use XPCSafeJSObjectWrapper. So this patch doesn't really that question either way.
By the way, I don't see how this crash could be anything but a null dereference crash... I'm not sure how we could try to read or write to any addresses after trying to deref null.
http://hg.mozilla.org/mozilla-central/rev/7de3a1cdeb25
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Duplicate of this bug: 352134
agree w/Gregory and mrbkap that this should always be a null-deref

> but I occasionally noticed subsequent invalid memory accesses so
> there may be additional corruption occurring.

If we find a testcase showing that it should be moved to a new bug, it's not likely to be related to the wrapper problem fixed here.
Group: core-security
Whiteboard: [sg:critical] → [sg:nse] null deref
Depends on: 494445
This caused bug 494445, so if it's ever landed on branches that patch should be taken too.
We probably don't need this on the 1.9.0 branch, the incidence of crashes at XPCNativeWrapper::GetNewOrUsed is very low (4 in the last week).

XPCWrappedNative::GetNewOrUsed is another story, 520 crashes in the last week on the 1.9.0 branch.
Whiteboard: [sg:nse] null deref → [sg:dos] null deref
Blocks: 502458
Created attachment 386940 [details] [diff] [review]
1.9.0 patch

Here's a rolled up patch for 1.9.0.
Attachment #386940 - Flags: superreview+
Attachment #386940 - Flags: review+
Attachment #386940 - Flags: approval1.9.0.12?
Blocking 1.9.0.12 for tracking purposes -- fixes regression bug 502458
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.12+
Comment on attachment 386940 [details] [diff] [review]
1.9.0 patch

Approved for 1.9.0.12, a=dveditz
Attachment #386940 - Flags: approval1.9.0.12? → approval1.9.0.12+
Checking in js/src/xpconnect/src/XPCNativeWrapper.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp,v  <--  XPCNativeWrapper.cpp
new revision: 1.82; previous revision: 1.81
done
Keywords: fixed1.9.0.12
Verified fixed in 1.9.0.12 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.12pre) Gecko/2009070606 GranParadiso/3.0.12pre (.NET CLR 3.5.30729).
Keywords: fixed1.9.0.12 → verified1.9.0.12
Keywords: fixed1.9.1
affects even 1.8.0.

Comment 20

9 years ago
Created attachment 387425 [details] [diff] [review]
Patch for 1.8.0 version

Attached patch for 1.8.0 version. Thanks for your review in advance.
Attachment #387425 - Flags: review?(mrbkap)

Comment 21

9 years ago
Created attachment 387431 [details] [diff] [review]
Patch for 1.8.0 version, rev2

Fix also issue in #494445. Please can you check it? TIA.
Attachment #387425 - Attachment is obsolete: true
Attachment #387431 - Flags: review?(mrbkap)
Attachment #387425 - Flags: review?(mrbkap)
Attachment #387431 - Flags: review?(mrbkap) → review+
You need to log in before you can comment on or make changes to this bug.