Bug 479560 (CVE-2009-1841)

Arbitrary code execution using sidebar

RESOLVED FIXED in mozilla1.9.2a1

Status

()

defect
P1
normal
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

Tracking

(4 keywords)

Trunk
mozilla1.9.2a1
Points:
---
Bug Flags:
blocking1.9.1 +
blocking1.9.0.11 +
wanted1.9.0.x +
blocking1.8.1.next +
wanted1.8.1.x +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical])

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
In nsSidebar.prototype.getInterfaces(), count is an object created in content,
thus it's unsafe to access count.value.

Updated

10 years ago
Whiteboard: [sg:critical]
Flags: blocking1.9.1?
Flags: blocking1.9.0.8?
Flags: blocking1.8.0.next?
Keywords: testcase
OS: Windows XP → All
Hardware: x86 → All
Assignee: nobody → mrbkap
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.2?
Flags: blocking1.9.0.8?
Flags: blocking1.9.0.8+
Flags: blocking1.8.1.next?
Flags: blocking1.8.0.next?
For what it's worth, I have no idea how we're going to fix this in a generic way on the 1.8 branch. This will be fixed on trunk and 1.9.1 by XPCUnsafeJSObjectWrapper (name still TBD).
Blake, is there a bug for XPCUnsafeJSObjectWrapper?  And, can you comment on whether or not we should block 1.9.1 for this?
(In reply to comment #3)
> Blake, is there a bug for XPCUnsafeJSObjectWrapper?  And, can you comment on
> whether or not we should block 1.9.1 for this?

I talked this over with jst, and we decided that this should block. The situation we have right now (whack-a-mole) will require about as much work as adding the new wrapper and will fix problems we don't even know about. I'll file a new bug on adding the new wrapper type.
Depends on: cow
Sorry, thought I did when I filed it -- fixed now.
Blocking, this could be bad. And this will likely be fixed by bug 480205.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Target Milestone: --- → mozilla1.9.1
Flags: blocking1.9.0.8+ → blocking1.9.0.9+
Posted patch FixSplinter Review
I don't know what to say.
Attachment #373256 - Flags: superreview?(jst)
Attachment #373256 - Flags: review?(jst)
Comment on attachment 373256 [details] [diff] [review]
Fix

Yeah, I hear ya :)
Attachment #373256 - Flags: superreview?(jst)
Attachment #373256 - Flags: superreview+
Attachment #373256 - Flags: review?(jst)
Attachment #373256 - Flags: review+
Landed: http://hg.mozilla.org/mozilla-central/rev/e31e971c0846
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.2a1
This is P1 now, as it's the actual required fix for the P1 bug 454363
Priority: P2 → P1
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1
Comment on attachment 373256 [details] [diff] [review]
Fix

assuming this code hasn't changed much, approved for 1.9.0.10, a=dveditz for release-drivers
Attachment #373256 - Flags: approval1.9.0.10+

Comment 16

10 years ago
Rob, we better test this in Firebug, it's the kind of thing that we break on.
(In reply to comment #16)
> Rob, we better test this in Firebug, it's the kind of thing that we break on.

We'll need to convert this testcase to something we can run in Firebug or, preferably FBTest. I can't even make sense of what this testcase is doing right now, but it's crazy.

Once we've got one, we'll have to verify it against Firefox ≤ 3.0.9, ≤ 3.5b3 and then vs. 3.0.10 and 3.5b4. I wish I'd seen this last month. :/

Also, this brings up the issue of how do we want to store our Firebug security tests? They could be bad-times if we just publish them in our usual tests repository. Probably not the ideal location for this discussion, but we don't have a Firebug SG either.
I guess the other possibility is that we just check for regressions in Firebug as a result of this checkin. Probably the easier alternative.
For what it's worth, the only way this patch could break Firebug is if it sets a setter on Object.prototype.value and then calls a double-wrapped function that takes an out parameter and expects the setter to be called...

Comment 20

10 years ago
(In reply to comment #18)
> I guess the other possibility is that we just check for regressions in Firebug
> as a result of this checkin. Probably the easier alternative.

Sorry, I meant: we should run FBTest on the nightly 3.0 before this patch ships.
(In reply to comment #20)
> (In reply to comment #18)
> > I guess the other possibility is that we just check for regressions in Firebug
> > as a result of this checkin. Probably the easier alternative.
> 
> Sorry, I meant: we should run FBTest on the nightly 3.0 before this patch
> ships.

ah, good call. Panic averted. :)
Checking in js/src/xpconnect/src/xpcprivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v  <--  xpcprivate.h
new revision: 1.284; previous revision: 1.283
done
Checking in js/src/xpconnect/src/xpcwrappedjsclass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp,v  <--  xpcwrappedjsclass.cpp
new revision: 1.118; previous revision: 1.117
done
Keywords: fixed1.9.0.10
(In reply to comment #17)
> Once we've got one, we'll have to verify it against Firefox ≤ 3.0.9, ≤ 3.5b3
> and then vs. 3.0.10 and 3.5b4. I wish I'd seen this last month. :/

Is anyone working on a new testcase so we can verify this patch?
Flags: in-testsuite?
Keywords: testcase-wanted
Target Milestone: mozilla1.9.1 → mozilla1.9.2a1
Version: unspecified → Trunk
(In reply to comment #23)
> Is anyone working on a new testcase so we can verify this patch?

You can verify this patch using the testcase here. Comment 17 is referring to creating a testcase that the Firebug guys can use.
I verified this fixed with the testcase here for 1.9.0.11 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.11pre) Gecko/2009051104 GranParadiso/3.0.11pre. I verified the bad behavior with 1.9.0.10.
Posted patch 1.8.0 patchSplinter Review
It's the same as 1.9 patch + there's JS_GetGlobalForObject() function from 1.9 line
Flags: blocking1.8.1.next?
Does this bug actually depend on bug 480205 ? I don't think it does with the fix we used ...
No, it doesn't.
No longer depends on: cow
Flags: blocking1.8.1.next?
Flags: blocking1.8.1.next? → blocking1.8.1.next+
Comment on attachment 377155 [details] [diff] [review]
1.8.0 patch

Approved for 1.8.1.22, a=dveditz for release-drivers
Attachment #377155 - Flags: approval1.8.1.next+
Checking in js/src/jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.214.2.45; previous revision: 3.214.2.44
done
Checking in js/src/jsapi.h;
/cvsroot/mozilla/js/src/jsapi.h,v  <--  jsapi.h
new revision: 3.107.2.13; previous revision: 3.107.2.12
done
Checking in js/src/xpconnect/src/xpcprivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v  <--  xpcprivate.h
new revision: 1.162.2.22; previous revision: 1.162.2.21
done
Checking in js/src/xpconnect/src/xpcwrappedjsclass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp,v  <--  xpcwrappedjsclass.cpp
new revision: 1.84.4.9; previous revision: 1.84.4.8
done
Keywords: fixed1.8.1.22
There is no way to test this for 1.8. There are no Firefox 1.8 builds anymore and it doesn't reproduce in the 1.8 Seamonkey builds.
(Reporter)

Comment 32

10 years ago
(In reply to comment #31)
> There is no way to test this for 1.8. There are no Firefox 1.8 builds anymore
> and it doesn't reproduce in the 1.8 Seamonkey builds.

I can reproduce the testcase on SeaMonkey-1.1.16.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.21) Gecko/20090403
SeaMonkey/1.1.16 -> Components.stack alert appears

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.22pre) Gecko/20090602
SeaMonkey/1.1.17pre -> no alert appears
Weird. My mistake then.

Marking as verified for 1.8.1.22.
Flags: blocking1.9.2?
Group: core-security
Alias: CVE-2009-1841
You need to log in before you can comment on or make changes to this bug.