Last Comment Bug 479560 - (CVE-2009-1841) Arbitrary code execution using sidebar
(CVE-2009-1841)
: Arbitrary code execution using sidebar
Status: RESOLVED FIXED
[sg:critical]
: fixed1.9.1, testcase, verified1.8.1.22, verified1.9.0.11
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.9.2a1
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
:
Mentors:
Depends on:
Blocks: 454363
  Show dependency treegraph
 
Reported: 2009-02-20 22:45 PST by moz_bug_r_a4
Modified: 2015-10-16 11:47 PDT (History)
15 users (show)
jst: blocking1.9.1+
samuel.sidler+old: blocking1.9.0.11+
dveditz: wanted1.9.0.x+
dveditz: blocking1.8.1.next+
dveditz: wanted1.8.1.x+
hskupin: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (2.29 KB, patch)
2009-04-16 20:21 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
jst: review+
jst: superreview+
dveditz: approval1.9.0.11+
Details | Diff | Review
1.8.0 patch (2.68 KB, patch)
2009-05-13 06:48 PDT, Martin Stránský
dveditz: approval1.8.1.next+
Details | Diff | Review

Description moz_bug_r_a4 2009-02-20 22:45:27 PST
In nsSidebar.prototype.getInterfaces(), count is an object created in content,
thus it's unsafe to access count.value.
Comment 2 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-02-23 13:47:52 PST
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).
Comment 3 Damon Sicore (:damons) 2009-02-24 11:49:17 PST
Blake, is there a bug for XPCUnsafeJSObjectWrapper?  And, can you comment on whether or not we should block 1.9.1 for this?
Comment 5 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-02-25 15:05:05 PST
(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.
Comment 7 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2009-02-25 18:34:03 PST
Please cc me.
Comment 8 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-02-25 18:39:34 PST
Sorry, thought I did when I filed it -- fixed now.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2009-02-26 16:42:58 PST
Blocking, this could be bad. And this will likely be fixed by bug 480205.
Comment 10 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-04-16 20:21:51 PDT
Created attachment 373256 [details] [diff] [review]
Fix

I don't know what to say.
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2009-04-16 20:24:18 PDT
Comment on attachment 373256 [details] [diff] [review]
Fix

Yeah, I hear ya :)
Comment 12 Dave Townsend [:mossop] 2009-04-17 04:43:58 PDT
Landed: http://hg.mozilla.org/mozilla-central/rev/e31e971c0846
Comment 13 Mike Beltzner [:beltzner, not reading bugmail] 2009-04-17 06:10:10 PDT
This is P1 now, as it's the actual required fix for the P1 bug 454363
Comment 14 Dave Townsend [:mossop] 2009-04-17 06:42:17 PDT
Fixed on the branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b2d7dbeb0f1b
Comment 15 Daniel Veditz [:dveditz] 2009-04-17 10:58:55 PDT
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
Comment 16 John J. Barton 2009-04-17 11:12:15 PDT
Rob, we better test this in Firebug, it's the kind of thing that we break on.
Comment 17 Rob Campbell [:rc] (:robcee) 2009-04-17 11:50:08 PDT
(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.
Comment 18 Rob Campbell [:rc] (:robcee) 2009-04-17 12:10:52 PDT
I guess the other possibility is that we just check for regressions in Firebug as a result of this checkin. Probably the easier alternative.
Comment 19 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-04-17 12:13:53 PDT
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 John J. Barton 2009-04-17 12:17:52 PDT
(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.
Comment 21 Rob Campbell [:rc] (:robcee) 2009-04-17 12:28:39 PDT
(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. :)
Comment 22 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-04-18 20:18:32 PDT
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
Comment 23 Henrik Skupin (:whimboo) 2009-04-21 07:18:23 PDT
(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?
Comment 24 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-04-28 13:05:52 PDT
(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.
Comment 25 Al Billings [:abillings] 2009-05-11 14:57:02 PDT
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.
Comment 26 Martin Stránský 2009-05-13 06:48:14 PDT
Created attachment 377155 [details] [diff] [review]
1.8.0 patch

It's the same as 1.9 patch + there's JS_GetGlobalForObject() function from 1.9 line
Comment 27 Mike Beltzner [:beltzner, not reading bugmail] 2009-05-22 09:43:30 PDT
Does this bug actually depend on bug 480205 ? I don't think it does with the fix we used ...
Comment 28 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-05-22 09:57:45 PDT
No, it doesn't.
Comment 29 Daniel Veditz [:dveditz] 2009-05-31 22:12:19 PDT
Comment on attachment 377155 [details] [diff] [review]
1.8.0 patch

Approved for 1.8.1.22, a=dveditz for release-drivers
Comment 30 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-06-01 17:53:26 PDT
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
Comment 31 Al Billings [:abillings] 2009-06-02 15:02:07 PDT
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.
Comment 32 moz_bug_r_a4 2009-06-02 23:34:16 PDT
(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
Comment 33 Al Billings [:abillings] 2009-06-03 11:39:49 PDT
Weird. My mistake then.

Marking as verified for 1.8.1.22.

Note You need to log in before you can comment on or make changes to this bug.