Last Comment Bug 504021 - (CVE-2010-0179) Arbitrary code execution with Firebug XMLHttpRequestSpy
(CVE-2010-0179)
: Arbitrary code execution with Firebug XMLHttpRequestSpy
Status: RESOLVED FIXED
[sg:critical][firebug-blocks] hold ad...
: testcase, verified1.9.0.19, verified1.9.1
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.9.3a1
Assigned To: Blake Kaplan (:mrbkap)
:
:
Mentors:
Depends on: 503926 541997
Blocks: JEP/caps 516911
  Show dependency treegraph
 
Reported: 2009-07-13 21:45 PDT by moz_bug_r_a4
Modified: 2010-05-09 15:45 PDT (History)
21 users (show)
mbeltzner: blocking1.9.2+
mbeltzner: blocking1.9.1.1-
dveditz: blocking1.9.0.18+
mbeltzner: blocking1.9.0.19+
dveditz: wanted1.9.0.x+
hskupin: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta4-fixed
.8+
.8-fixed


Attachments
wip (1.06 KB, patch)
2009-07-15 13:10 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
wip 2 (31.10 KB, patch)
2009-08-19 15:54 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
patch v1 (34.77 KB, patch)
2009-08-19 17:31 PDT, Blake Kaplan (:mrbkap)
jst: review+
bzbarsky: review+
dveditz: superreview+
Details | Diff | Splinter Review
Applying to the 1.9.2 branch (36.89 KB, patch)
2009-11-06 07:38 PST, Blake Kaplan (:mrbkap)
jst: review+
Details | Diff | Splinter Review
As promised (10.63 KB, patch)
2009-11-06 07:41 PST, Blake Kaplan (:mrbkap)
jst: review+
Details | Diff | Splinter Review
Applying to the 1.9.1 branch -- but untested (36.92 KB, patch)
2009-11-06 09:30 PST, Blake Kaplan (:mrbkap)
jst: review+
mbeltzner: approval1.9.1.8+
Details | Diff | Splinter Review
Ibid (10.83 KB, patch)
2009-11-06 09:31 PST, Blake Kaplan (:mrbkap)
jst: review+
mbeltzner: approval1.9.1.8+
Details | Diff | Splinter Review
For 1.9.0 (22.54 KB, patch)
2010-01-26 16:25 PST, Blake Kaplan (:mrbkap)
jst: review+
dveditz: approval1.9.0.18+
Details | Diff | Splinter Review
Fix for comment 55 (4.02 KB, patch)
2010-02-05 15:02 PST, Blake Kaplan (:mrbkap)
jst: review+
mbeltzner: approval1.9.0.19+
Details | Diff | Splinter Review

Description moz_bug_r_a4 2009-07-13 21:45:21 PDT
spy.js:
top.XMLHttpRequestSpy.prototype =
{
    attach: function()
    {
        ...
        this.onreadystatechange = this.xhrRequest.onreadystatechange;

When accessing this.xhrRequest.onreadystatechange, content functions 
(QueryInterface, getInterfaces, etc.) can be called.
Comment 2 Blake Kaplan (:mrbkap) 2009-07-14 00:17:48 PDT
I'll take a look at this.
Comment 3 Rob Campbell [:rc] (:robcee) 2009-07-14 05:56:07 PDT
ugh. thanks for cc'ing me, ss.
Comment 4 Rob Campbell [:rc] (:robcee) 2009-07-14 05:57:01 PDT
cc'ing honza as he's familiar with the spy.js code
Comment 5 Blake Kaplan (:mrbkap) 2009-07-14 21:39:50 PDT
This is kind of rough. The call stack (as moz_bug_r_a4 probably already knows is):

<firebug JS>
XPC_NW_GetProperty(onreadystatechange)
...
C++2JSObject(XPCWrappedJS(moz_bug_r_a4's evil object))
...
XPCWrappedNativeProto::GetNewOrUsed
...
XPCWrappedJS::CallMethod("getInterfaces")
...
nsLocation::SetHref

So, even with bug 503926, we'd still have to deal with this. We could go one further in that bug and disable any sort of nsIClassInfo probing on content objects while double-wrapping them and that *would* stop this. I'm not sure how much I like that solution, though.

My other option would be, in nsXPCWrappedJSClass::CallMethod, try to pin caps' conception of the subject principal to the principal of the function object we're calling (whether or not it's a native function). I *think* that with the wrapperization we do now, with the exception of document.domain stuff, the function object in question is guaranteed to have the right principal.

Thoughts?
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2009-07-14 22:59:00 PDT
> try to pin caps' conception of the subject principal to the principal of the
> function object we're calling

That would also fix other issues we've run into, no?  With JS-implemented components not being callable into from C++ if there's content JS on the stack?
Comment 7 Blake Kaplan (:mrbkap) 2009-07-15 13:10:45 PDT
Created attachment 388767 [details] [diff] [review]
wip

This works, but I'm really worried about performance.
Comment 8 Blake Kaplan (:mrbkap) 2009-07-15 20:47:36 PDT
Comment on attachment 388767 [details] [diff] [review]
wip

One thing I probably want to do is unwrap aJSObj so we don't get SJOW(SJOW(...)) or something.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2009-07-15 21:47:30 PDT
I'm not sure we have too many really perf-sensitive uses of XPCWrappedJS, do we? I certainly hope not!

That said, how would this patch help?
Comment 10 Blake Kaplan (:mrbkap) 2009-07-15 23:18:20 PDT
(In reply to comment #9)
> I'm not sure we have too many really perf-sensitive uses of XPCWrappedJS, do
> we? I certainly hope not!

Event handlers spring to mind. smaug did a bunch of work in bug 461861 to reduce the perf impact of firing off events.

> That said, how would this patch help?

Setting onreadystatechange wraps the function (or event handler, whichever) in an nsXPCWrappedJS. The bug that moz_bug_r_a4 has found is that when Firebug tries to get a reference to onreadystatechange, we call getInterfaces on the existing handler (through the nsXPCWrappedJS). So if that is a native function, we have a bug 344495-style attack. The SJOW created here sits between the Firebug code and the code that is wrapping the handler, preventing the attack.

Another reason I'm not terribly happy with this patch is that, with the patch, when we actually return the double-wrapped object, we end up with SJOW(XPCWrappedNative(nsXPCWrappedJS(SJOW(actual object)))) which is really, really excessive.

This is technically a regression from bug 479560 since before that bug, the page had no way of controlling the argument to the native function.
Comment 11 moz_bug_r_a4 2009-07-15 23:41:03 PDT
(In reply to comment #5)
> <firebug JS>
> XPC_NW_GetProperty(onreadystatechange)
> ...
> C++2JSObject(XPCWrappedJS(moz_bug_r_a4's evil object))
> ...
> XPCWrappedNativeProto::GetNewOrUsed
> ...
> XPCWrappedJS::CallMethod("getInterfaces")
> ...
> nsLocation::SetHref
> 
> So, even with bug 503926, we'd still have to deal with this. We could go one
> further in that bug and disable any sort of nsIClassInfo probing on content
> objects while double-wrapping them and that *would* stop this. I'm not sure how
> much I like that solution, though.

For what it's worth, the testcase depends on the fact that my evil object can
be QIed to nsIClassInfo.  If my evil object does not have QueryInterface() then
getInterfaces() is not called.
Comment 12 Blake Kaplan (:mrbkap) 2009-07-15 23:57:39 PDT
(In reply to comment #11)
> For what it's worth, the testcase depends on the fact that my evil object can
> be QIed to nsIClassInfo.  If my evil object does not have QueryInterface() then
> getInterfaces() is not called.

Ah, true...
Comment 13 moz_bug_r_a4 2009-07-16 00:20:33 PDT
(In reply to comment #10)
> This is technically a regression from bug 479560 since before that bug, the
> page had no way of controlling the argument to the native function.

I'll attach another testcase that does not depend on the fix for bug 479560.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2009-07-16 04:56:12 PDT
> Event handlers spring to mind.

Sure.  The current cost of XPCWrappedJS dispatch is already a large fraction of that cost, though.  Does this really significantly slow down that part?  It's not particularly pretty as it is....

> The SJOW created here sits between the Firebug code and the code that is
> wrapping the handler

I guess I'm not clear on where it sits exactly.  Does it sit between the XPCWrappedJS and the untrusted JSObject?
Comment 16 Blake Kaplan (:mrbkap) 2009-07-16 09:21:43 PDT
(In reply to comment #15)
> Sure.  The current cost of XPCWrappedJS dispatch is already a large fraction of
> that cost, though.  Does this really significantly slow down that part?  It's
> not particularly pretty as it is....

Well, I might actually test and find that it doesn't matter, I was just worried. Doing anything through a SJOW requires compiling JS, which isn't the fastest thing in the world.

> I guess I'm not clear on where it sits exactly.  Does it sit between the
> XPCWrappedJS and the untrusted JSObject?

Yes.
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2009-07-16 09:54:05 PDT
Ah, I see.  OK, I see why it fixes the bug now.  ;)

Given that change, can we stop doing the explicit security checks XPCWrappedJS does now?  Or not so much?
Comment 18 Blake Kaplan (:mrbkap) 2009-08-19 15:54:37 PDT
Created attachment 395437 [details] [diff] [review]
wip 2

This won't fix this bug, but it's almost there!
Comment 19 Blake Kaplan (:mrbkap) 2009-08-19 17:31:42 PDT
Created attachment 395462 [details] [diff] [review]
patch v1

This patch fixes "testcase" if I back the patch for bug 503926 out of my tree. "testcase 2" still works because it goes directly through nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject and I didn't bother patching that function precisely because of the patch for bug 503926.

I think we probably want to be using the new nsIScriptSecurityManager API in more places. In particular, places that we push contexts and set aside frame chains (in part because there's a weird interaction between setting aside a frame chain and the API added here, see the comment in nsScriptSecurityManager::GetPrincipalAndFrame).
Comment 20 Blake Kaplan (:mrbkap) 2009-08-19 17:32:57 PDT
Oh, still to come: tests for XPCSafeJSObjectWrapper.
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2009-08-24 08:50:10 PDT
Comment on attachment 395462 [details] [diff] [review]
patch v1

>+        ContextPrincipal *next;
>+        JSContext *cx;
>+        JSStackFrame *fp;
>+        nsCOMPtr<nsIPrincipal> principal;

mMember, please.

>+++ b/caps/src/nsScriptSecurityManager.cpp
>+nsScriptSecurityManager::PopContextPrincipal(JSContext *cx)

Why do we even want the cx arg here?

What's the frame that GetPrincipalAndFrame returns used for?  Why do we want the innermost frame in the clamped case?

Why do we need that debug noise printf?  should it be DEBUG_mrbkap or something?

The last looks fine.  Will make those annoying security error messages in the manually compiled functions go away too.  ;)
Comment 22 Blake Kaplan (:mrbkap) 2009-08-26 16:40:20 PDT
(In reply to comment #21)
> mMember, please.

Ew. But OK.

> >+nsScriptSecurityManager::PopContextPrincipal(JSContext *cx)
> 
> Why do we even want the cx arg here?

I sort of like having the sanity check that we're properly stack-like in pushing and popping. I can remove it if you don't think it's worth it.

> What's the frame that GetPrincipalAndFrame returns used for?  Why do we want
> the innermost frame in the clamped case?

Oops, I meant:

            if (fp && fp == target)
            {
                *frameResult = fp;
            }
            else
            {
                JSStackFrame *inner = nsnull;
                *frameResult = JS_FrameIterator(cx, &inner);
            }

> Why do we need that debug noise printf?  should it be DEBUG_mrbkap or
> something?

Nah, I nuked it. I was a little worried that the list would get very long. Some light testing that I did showed that it doesn't.

> The last looks fine.  Will make those annoying security error messages in the
> manually compiled functions go away too.  ;)

Hmm, the error messages won't be misleading, but we're not going to have source coordinates anymore (because we set aside the frame chain). I don't think this is any worse than what we have now, but I don't think it's better.
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2009-08-26 16:56:11 PDT
> Oops, I meant:

Ah, ok.  That makes a lot more sense.  ;)
Comment 24 Johnathan Nightingale [:johnath] 2009-09-21 06:21:13 PDT
This one's been asleep for a month or so - Johnny, Dan - little review love?
Comment 25 Daniel Veditz [:dveditz] 2009-09-24 07:32:20 PDT
Comment on attachment 395462 [details] [diff] [review]
patch v1

>+    [noscript] void pushContextPrincipal(in JSContextPtr cx,
>+    [noscript] void popContextPrincipal(in JSContextPtr cx);

We should add new functions to the end of the interface, not in the middle please.

sr=dveditz on the caps stuff. I briefly looked at the xpconnect part but not closely enough to call a sr+
Comment 26 Johnny Stenback (:jst, jst@mozilla.com) 2009-09-24 17:32:41 PDT
Comment on attachment 395462 [details] [diff] [review]
patch v1

- In nsScriptSecurityManager.h:

-    static nsIPrincipal*
+    nsIPrincipal*
     GetSubjectPrincipal(JSContext* cx, nsresult* rv);

I wonder if this performs better, worse, or the same, as leaving these static and making the members they now need also static? Performance used to matter a lot here, but probably matters way less nowadays.

- In nsScriptSecurityManager::GetPrincipalAndFrame():

         // Get principals from innermost frame of JavaScript or Java.

Java, really? :) Wanna fix that comment while you're here?

r=jst
Comment 27 Blake Kaplan (:mrbkap) 2009-09-30 19:46:42 PDT
http://hg.mozilla.org/mozilla-central/rev/5c503628b3ed
Comment 28 Rob Campbell [:rc] (:robcee) 2009-10-29 10:52:06 PDT
any approval love for this for 1.9.2?
Comment 29 Rob Campbell [:rc] (:robcee) 2009-10-29 11:18:13 PDT
needs checkin. Johnath clarified the flagging process.
Comment 30 Rob Campbell [:rc] (:robcee) 2009-11-03 12:47:51 PST
Comment on attachment 395462 [details] [diff] [review]
patch v1

I still don't see any checkin notices in the bug. Rerequesting approval (this time actually setting the flag)...
Comment 31 Samuel Sidler (old account; do not CC) 2009-11-04 17:44:18 PST
Note that this changes an interface, which I'm not sure we can do for 1.9.2 now?

Blake: Where are we on getting this landed on 1.9.2 and getting a 1.9.1 patch ready?
Comment 32 Blake Kaplan (:mrbkap) 2009-11-06 07:38:53 PST
Created attachment 410788 [details] [diff] [review]
Applying to the 1.9.2 branch

I think this is a proper backport of this patch to the 1.9.2 branch. I'll do some testing in a second. Note: DO NOT LAND THIS ON THE 1.9.2 BRANCH. I'm going to attach another patch, on top of this one that makes this safe to land on a branch where we're not allowed to change nsIScriptSecurityManager.
Comment 33 Blake Kaplan (:mrbkap) 2009-11-06 07:41:26 PST
Created attachment 410791 [details] [diff] [review]
As promised
Comment 34 Blake Kaplan (:mrbkap) 2009-11-06 09:30:00 PST
Created attachment 410808 [details] [diff] [review]
Applying to the 1.9.1 branch -- but untested

This is the same deal as before...
Comment 35 Blake Kaplan (:mrbkap) 2009-11-06 09:31:35 PST
Created attachment 410809 [details] [diff] [review]
Ibid
Comment 36 Blake Kaplan (:mrbkap) 2009-11-06 10:25:45 PST
Note that the 1.9.1 branch patch probably accidentally contains the fix for bug 508752, which isn't really correct.
Comment 37 Daniel Veditz [:dveditz] 2009-11-10 17:07:13 PST
And the 1.9.1 patch does NOT contain the fixes for dependencies and regressions bug 503926, bug 502959 (by way of bug 506752), and bug 509824. That's a lot of patching to do with Blake not around :-(

I think we've got to wait on this one on the older branches.

Also you've changed the IID of an interface that says not to change it without coordinating with a new JEP update, and we had to go back to using the JEP on 1.9.2,  and we definitely don't want to be doing it that way on a stable branch (hello _MOZILLA_1.9.1_BRANCH interface ugliness).
Comment 38 Blake Kaplan (:mrbkap) 2009-11-17 08:43:40 PST
(In reply to comment #37)
> And the 1.9.1 patch does NOT contain the fixes for dependencies and regressions
> bug 503926, bug 502959 (by way of bug 506752), and bug 509824. That's a lot of
> patching to do with Blake not around :-(

bug 503926 isn't a regression from this bug. That bug happened to fix this exploit, leading to a "depends on" relationship. I don't know whether we want to take it on the 1.9.1 branch. We can decide if we want to take that fix separately from this one (thanks for letting us make the distinction, bugzilla!).

> Also you've changed the IID of an interface

The first patch changes the IID and the second one (Ibid) changes it back. I did it that way to maintain my own sanity when dealing with these patch as well as to make reviewing the patches a little easier.
Comment 39 Blake Kaplan (:mrbkap) 2009-11-18 07:15:29 PST
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6ccaf1dd6553
Comment 40 Johnny Stenback (:jst, jst@mozilla.com) 2010-01-13 17:42:26 PST
Comment on attachment 410808 [details] [diff] [review]
Applying to the 1.9.1 branch -- but untested

Looks good.

r=jst
Comment 41 Mike Beltzner [:beltzner, not reading bugmail] 2010-01-15 13:26:27 PST
Sorry, Blake: which of these patches are we supposed to be approving/landing? The one nominated, or the one marked ibid?
Comment 42 Blake Kaplan (:mrbkap) 2010-01-15 13:32:17 PST
(In reply to comment #41)
> Sorry, Blake: which of these patches are we supposed to be approving/landing?
> The one nominated, or the one marked ibid?

They have to land together: attachment 410808 [details] [diff] [review] is a direct backport of the patch that landed on trunk to the 1.9.1 branch and attachment 410809 [details] [diff] [review] makes it safe to land on the old branch, where we're not allowed to change interfaces.
Comment 43 Mike Beltzner [:beltzner, not reading bugmail] 2010-01-19 10:10:14 PST
Comment on attachment 410808 [details] [diff] [review]
Applying to the 1.9.1 branch -- but untested

a=beltzner for 191 branch
Comment 44 Daniel Veditz [:dveditz] 2010-01-19 10:11:48 PST
Is the 1.9.0 branch immune, or will we be 0-daying ourselves by releasing this fix on the 1.9.1/1.9.2 branches?
Comment 45 Blake Kaplan (:mrbkap) 2010-01-19 14:12:20 PST
(In reply to comment #44)
> Is the 1.9.0 branch immune, or will we be 0-daying ourselves by releasing this
> fix on the 1.9.1/1.9.2 branches?

Yeah, this affects the 1.9.0 branch. However, on that branch, I think that we should just take bug 503926, which fixes this bug with far less code churn (though, admittedly, less thoroughly).
Comment 46 Blake Kaplan (:mrbkap) 2010-01-21 13:29:29 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2ad3dc1dcf42
Comment 47 Daniel Veditz [:dveditz] 2010-01-22 13:10:36 PST
(In reply to comment #45)
> Yeah, this affects the 1.9.0 branch. However, on that branch, I think that we
> should just take bug 503926, which fixes this bug with far less code churn

Fine, marked that bug and its regressions as blocking 1.9.0.18. Still going to make this a blocker because this is the security hole you claim is fixed by that other one.
Comment 48 Blake Kaplan (:mrbkap) 2010-01-26 16:25:51 PST
Created attachment 423691 [details] [diff] [review]
For 1.9.0

jst, want to give this a once-over?
Comment 49 Blake Kaplan (:mrbkap) 2010-01-26 16:42:47 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ca3f3f6b9846 backs out the SJOW stuff on 1.9.1. Hopefully that'll fix the fallout from that changeset on the 1.9.1 branch.
Comment 50 Daniel Veditz [:dveditz] 2010-01-26 17:13:19 PST
Comment on attachment 423691 [details] [diff] [review]
For 1.9.0

Approved for 1.9.0.18, a=dveditz
Comment 51 Blake Kaplan (:mrbkap) 2010-01-26 17:22:16 PST
Checking in caps/idl/nsIScriptSecurityManager.idl;
/cvsroot/mozilla/caps/idl/nsIScriptSecurityManager.idl,v  <--  nsIScriptSecurityManager.idl
new revision: 1.81; previous revision: 1.80
done
Checking in caps/include/nsScriptSecurityManager.h;
/cvsroot/mozilla/caps/include/nsScriptSecurityManager.h,v  <--  nsScriptSecurityManager.h
new revision: 1.118; previous revision: 1.117
done
Checking in caps/src/nsScriptSecurityManager.cpp;
/cvsroot/mozilla/caps/src/nsScriptSecurityManager.cpp,v  <--  nsScriptSecurityManager.cpp
new revision: 1.366; previous revision: 1.365
done
Checking in js/src/xpconnect/shell/xpcshell.cpp;
/cvsroot/mozilla/js/src/xpconnect/shell/xpcshell.cpp,v  <--  xpcshell.cpp
new revision: 1.113; previous revision: 1.112
done
Checking in js/src/xpconnect/src/XPCWrapper.h;
/cvsroot/mozilla/js/src/xpconnect/src/XPCWrapper.h,v  <--  XPCWrapper.h
new revision: 1.16; previous revision: 1.15
done
Checking in js/src/xpconnect/src/nsXPConnect.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/nsXPConnect.cpp,v  <--  nsXPConnect.cpp
new revision: 1.176; previous revision: 1.175
done
Checking in js/src/xpconnect/src/xpcwrappedjsclass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp,v  <--  xpcwrappedjsclass.cpp
new revision: 1.119; previous revision: 1.118
done
Comment 52 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2010-01-28 13:39:57 PST
I have problems to reproduce this bug with Firefox 3.5.7. Means testcase 1 and 2 don't show an alert dialog for me when loading and Firebug's console enabled. Is there something else I would have to prepare?
Comment 53 moz_bug_r_a4 2010-01-28 17:18:18 PST
Are you using Firebug 1.5.0?  It seems that Firebug 1.5.0 distinguishes fx3.5
form fx3.6, and on fx3.5 it does not run code that triggers this bug.

I can reproduce testcase 1 & 2 on fx3.5.7 with Firebug 1.4.5.

https://addons.mozilla.org/en-US/firefox/addons/versions/1843#version-1.4.5
Comment 54 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2010-01-29 06:28:38 PST
Good hint! I had Firebug 1.5 installed. So I did a check with Firebug 1.4.5 again and I'm able to see the issue now.

By running both of the testcases with a recent Shiretoko build on all platforms doesn't bring up the alerts anymore. Marking as verified fixed with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8pre) Gecko/20100128 Shiretoko/3.5.8pre (.NET CLR 3.5.30729) ID:20100128044603
Comment 55 moz_bug_r_a4 2010-02-03 18:30:03 PST
testcase 2 still works on 1.9.0 with Firebug 1.4.5 (note: Firebug 1.5.0 does
not support fx3.0.*).

Patches for this bug can fix testcase 1 but cannot fix testcase 2 as described
in comment 19.  Actually both testcase 1 and 2 were fixed by bug 503926 on
trunk/1.9.2/1.9.1, but that bug is not fixed on 1.9.0 yet.
Comment 56 Blake Kaplan (:mrbkap) 2010-02-05 15:02:24 PST
Created attachment 425570 [details] [diff] [review]
Fix for comment 55

I missed this because it wasn't needed on the newer branches -- the added code was belt-and-braces. *sigh*.
Comment 57 Al Billings [:abillings] 2010-02-10 17:51:59 PST
We'll need to include Blake's update for 1.9.0.19. 

Dan Veditz and Beltzner, do you agree?
Comment 58 Al Billings [:abillings] 2010-02-10 17:52:52 PST
I suppose that I should CC Beltzner if I want a comment on comment 57 soon.
Comment 59 Mike Beltzner [:beltzner, not reading bugmail] 2010-02-12 13:06:17 PST
Removing fixed1.9.0.18 flag, marking as blocking.
Comment 60 Daniel Veditz [:dveditz] 2010-02-16 10:55:04 PST
Comment on attachment 425570 [details] [diff] [review]
Fix for comment 55

Is this ready to land for 1.9.0.19?
Comment 61 Blake Kaplan (:mrbkap) 2010-02-16 11:32:28 PST
Yes.
Comment 62 Mike Beltzner [:beltzner, not reading bugmail] 2010-02-22 10:12:48 PST
Comment on attachment 425570 [details] [diff] [review]
Fix for comment 55

a=beltzner for 1.9.0 branch
Comment 63 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-03 14:45:09 PST
We can haz branch landing?
Comment 64 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-08 20:50:52 PST
mrbkap, are we holding off on landing this for any reason?
Comment 65 Blake Kaplan (:mrbkap) 2010-03-10 13:11:32 PST
Checking in js/src/xpconnect/src/xpcwrappedjsclass.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp,v  <--  xpcwrappedjsclass.cpp
new revision: 1.120; previous revision: 1.119
done
Comment 66 Al Billings [:abillings] 2010-03-12 15:58:49 PST
Verified that testcase 2 is fixed in 1.9.0 now with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.19pre) Gecko/2010031106 GranParadiso/3.0.19pre (.NET CLR 3.5.30729) and Firebug 1.4.5.

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