As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
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)
:
: David Keeler [:keeler] (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)
jst: review+
jst: superreview+
dveditz: approval1.9.0.11+
Details | Diff | Splinter 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 | Splinter Review

Description User image 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 User image Blake Kaplan (:mrbkap) 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 User image 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 User image Blake Kaplan (:mrbkap) 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 User image Mano (::mano, needinfo? for any questions; not reading general bugmail) 2009-02-25 18:34:03 PST
Please cc me.
Comment 8 User image Blake Kaplan (:mrbkap) 2009-02-25 18:39:34 PST
Sorry, thought I did when I filed it -- fixed now.
Comment 9 User image 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 User image Blake Kaplan (:mrbkap) 2009-04-16 20:21:51 PDT
Created attachment 373256 [details] [diff] [review]
Fix

I don't know what to say.
Comment 11 User image 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 User image Dave Townsend [:mossop] 2009-04-17 04:43:58 PDT
Landed: http://hg.mozilla.org/mozilla-central/rev/e31e971c0846
Comment 13 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Blake Kaplan (:mrbkap) 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 User image 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 User image 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 User image Blake Kaplan (:mrbkap) 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 User image 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 User image Blake Kaplan (:mrbkap) 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 User image 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 User image 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 User image 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 User image Blake Kaplan (:mrbkap) 2009-05-22 09:57:45 PDT
No, it doesn't.
Comment 29 User image 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 User image Blake Kaplan (:mrbkap) 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 User image 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 User image 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 User image 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.