Last Comment Bug 505988 - (CVE-2009-3374) It's possible to access a double-wrapped object's underlying privileged object
(CVE-2009-3374)
: It's possible to access a double-wrapped object's underlying privileged object
Status: RESOLVED FIXED
[sg:critical]
: verified1.9.0.15, verified1.9.1
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: unspecified
: x86 Windows XP
: P1 normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
:
:
Mentors:
Depends on: 508471 508483 509583
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-23 06:44 PDT by moz_bug_r_a4
Modified: 2009-12-15 10:58 PST (History)
9 users (show)
benjamin: blocking1.9.2+
dveditz: blocking1.9.0.15+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
roc: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed
.4+
.4-fixed


Attachments
Work in progress (8.71 KB, patch)
2009-07-23 17:43 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Proposed fix (9.11 KB, patch)
2009-07-24 15:20 PDT, Blake Kaplan (:mrbkap)
bzbarsky: review+
jst: superreview+
dveditz: approval1.9.1.4+
dveditz: approval1.9.0.15+
Details | Diff | Splinter Review

Description moz_bug_r_a4 2009-07-23 06:44:08 PDT
XPCVariant::VariantDataToJS() unwraps double-wrapped objects even when it
shouldn't.
Comment 1 moz_bug_r_a4 2009-07-23 06:45:56 PDT
Created attachment 390218 [details]
testcase - Arbitrary code execution
Comment 2 Samuel Sidler (old account; do not CC) 2009-07-23 10:29:04 PDT
Do we need this on 1.9.0? This definitely block 1.9.1.x but not sure if it's for .2 or .3.
Comment 3 Daniel Veditz [:dveditz] 2009-07-23 11:06:06 PDT
Yes, this affects Firefox 3.0.x
Comment 4 Blake Kaplan (:mrbkap) 2009-07-23 17:43:20 PDT
Created attachment 390374 [details] [diff] [review]
Work in progress

This patch fixes this bug, but I'm seeing spurious errors in the livemark service with it applied.
Comment 5 Blake Kaplan (:mrbkap) 2009-07-24 15:20:14 PDT
Created attachment 390563 [details] [diff] [review]
Proposed fix

The spurious errors were due to my incorrectly backing out bug 384632.
Comment 6 Daniel Veditz [:dveditz] 2009-07-24 15:45:38 PDT
The blocking1.9.0.13+ is a soft block, we'll take it on the 1.9.0 branch when it gets fixed on the 1.9.1 branch.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2009-07-24 15:47:30 PDT
Comment on attachment 390563 [details] [diff] [review]
Proposed fix

Seems ok.
Comment 8 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-30 03:17:26 PDT
http://hg.mozilla.org/mozilla-central/rev/03c40c5a2d4b
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2009-07-30 08:03:47 PDT
This caused a bunch of password manager test failures along these lines:

/test/test_basic_form_observer_autofillForms.html | Checking foundLogins is array - got "Object", expected "Array"
82398 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr
82407 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr/test/test_basic_form_observer_foundLogins.html | Checking foundLogins is array - got "Object", expected "Array"

and various other stuff, but I suspect most of it is due to the tests that got the errors above failing and due to the fact that the tests in this dir all depend on each other(!).  Which means you have to run with

   --test-path=toolkit/components/passwordmgr/test/

to test reasonably; just running a single test won't work.  And if you run a single test and then the whole dir that will also fail...

It's not clear to me whether this is a problem in the test or in the patch yet, but for now backed out to get the tree green: http://hg.mozilla.org/mozilla-central/rev/c1ab8650e0ce
Comment 10 Mike Beltzner [:beltzner, not reading bugmail] 2009-08-04 04:44:26 PDT
This is a P1 1.9.2 blocker, thus blocking Firefox 3.6a1 - any ETA on re-landing or analysing why this caused so many test failures?
Comment 11 Mike Beltzner [:beltzner, not reading bugmail] 2009-08-04 12:12:35 PDT
Justin: can you take a second to see if you can figure out why the PW Mgr tests failed?
Comment 12 Justin Dolske [:Dolske] 2009-08-04 13:47:09 PDT
So, this was a  test added for bug 492153. That bug added notifications for when pwmgr does things when processing a form. The notification's subject is a nsIPropertyBag, with various properties set. One of these is a JS array of relevant logins.

The notification is built in nsLoginManager.js's _notifyFoundLogins() as

  propbag.setPropertyAsInterface("foundLogins", foundLogins.concat());

And the test is checking

  propbag.get("foundLogins").constructor.name, "Array", "Checking foundLogins is array");

I'd guess that either something is going wrong in setting the property (I think there's some XPCOM magic being used to shove an Array into an nsISupports param), or that checking .constructor.name isn't a reliable thing to do. [ISTR Firebug having run into similar problems.]

mrbkap, does this help?
Comment 13 Mike Beltzner [:beltzner, not reading bugmail] 2009-08-05 07:02:08 PDT
I need to know if this needs to land for alpha 1 in order to make 1.9.2; it's literally holding our alpha production and branch at this point.

Please respond ASAP.
Comment 14 Blake Kaplan (:mrbkap) 2009-08-05 10:49:31 PDT
(In reply to comment #13)
> I need to know if this needs to land for alpha 1 in order to make 1.9.2; it's
> literally holding our alpha production and branch at this point.
> 
> Please respond ASAP.

Sorry, I should have commented here... The reason for the MochiTest failures are now blocking this bug. Once those bugs land, this one should be able to as well.
Comment 15 Daniel Veditz [:dveditz] 2009-08-05 16:36:37 PDT
This probably needs to wait for the next round of releases rather than trying to rush this into 1.9.1.3/1.9.0.14
Comment 16 Blake Kaplan (:mrbkap) 2009-08-05 19:35:39 PDT
http://hg.mozilla.org/mozilla-central/rev/5c913c4662d8
Comment 17 Mike Beltzner [:beltzner, not reading bugmail] 2009-08-25 10:40:01 PDT
Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)
Comment 18 Daniel Veditz [:dveditz] 2009-09-10 11:14:56 PDT
This is blocking 1.9.1.4 and 1.9.0.15 (code freeze Sept 22!). We probably need a branch patch that includes any regression fixes, or we need branch approval requests on those bugs too.
Comment 19 Daniel Veditz [:dveditz] 2009-09-22 17:02:20 PDT
Comment on attachment 390563 [details] [diff] [review]
Proposed fix

Approved for 1.9.1.4 and 1.9.0.15, a=dveditz
Comment 20 Blake Kaplan (:mrbkap) 2009-09-22 17:10:58 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ed60afa878e9

Checking in js/src/xpconnect/src/xpcprivate.h;
/cvsroot/mozilla/js/src/xpconnect/src/xpcprivate.h,v  <--  xpcprivate.h
new revision: 1.286; previous revision: 1.285
done
Checking in js/src/xpconnect/src/xpcvariant.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/xpcvariant.cpp,v  <--  xpcvariant.cpp
new revision: 1.32; previous revision: 1.31
done
Checking in js/src/xpconnect/tests/mochitest/Makefile.in;
/cvsroot/mozilla/js/src/xpconnect/tests/mochitest/Makefile.in,v  <--  Makefile.in
new revision: 1.12; previous revision: 1.11
done
RCS file: /cvsroot/mozilla/js/src/xpconnect/tests/mochitest/test_bug384632.html,v
done
Checking in js/src/xpconnect/tests/mochitest/test_bug384632.html;
/cvsroot/mozilla/js/src/xpconnect/tests/mochitest/test_bug384632.html,v  <--  test_bug384632.html
initial revision: 1.1
done
Comment 21 Al Billings [:abillings] 2009-09-28 10:30:35 PDT
Verified fixed for 1.9.0.15 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.15pre) Gecko/2009092805 GranParadiso/3.0.15pre (.NET CLR 3.5.30729). 

Verified fixed for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090924 Shiretoko/3.5.4pre (.NET CLR 3.5.30729).
Comment 22 Daniel Veditz [:dveditz] 2009-10-23 16:05:17 PDT
This testcase doesn't run on the 1.8 branch because I get NS_ERROR_NOT_IMPLEMENTED from the document.setUserData() call. Is there some way to get the double unwrapping to happen on that branch? Or should we just not bother because that branch is missing so many wrappers anyway?
Comment 23 moz_bug_r_a4 2009-10-24 05:03:53 PDT
It seems that this bug is a regression from bug 384632 and that bug is not
fixed on the 1.8 branch and thus the 1.8 branch is not affected by this bug.
Comment 24 moz_bug_r_a4 2009-10-24 05:05:05 PDT
Created attachment 408197 [details]
testcase 2 - Arbitrary code execution

This testcase uses XSLTProcessor.setParameter().  This testcase works on
fx-3.5.3/fx-3.0.14 and is fixed on fx-3.5.4/fx-3.0.15.  On SeaMonkey-1.1.18, 
the same behavior as fx-3.5.4/fx-3.0.15.

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