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

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.

Please cc me.

Sorry, thought I did when I filed it -- fixed now.

Blocking, this could be bad. And this will likely be fixed by bug 480205.

I don't know what to say.

Comment on attachment 373256 [details] [diff] [review] Fix Yeah, I hear ya :)

Landed: http://hg.mozilla.org/mozilla-central/rev/e31e971c0846

This is P1 now, as it's the actual required fix for the P1 bug 454363

Fixed on the branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b2d7dbeb0f1b

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

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...

(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

(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?

(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.

It's the same as 1.9 patch + there's JS_GetGlobalForObject() function from 1.9 line

Does this bug actually depend on bug 480205 ? I don't think it does with the fix we used ...

No, it doesn't.

Comment on attachment 377155 [details] [diff] [review] 1.8.0 patch Approved for 1.8.1.22, a=dveditz for release-drivers

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

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.

(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.