399298 - Bypassing XPCNativeWrapper by redefining XPCNativeWrapper

Status: VERIFIED FIXED

(Core :: Security, defect, P1)

Description

[moz_bug_r_a4]

When creating an XPCNativeWrapper object, we get the prototype object for it
from content global scope.  But, content can override the prototype object by
redefining XPCNativeWrapper.

XPCNativeWrapper = function() {};
XPCNativeWrapper.prototype = x;

Comment 1

[moz_bug_r_a4]

Attachment: testcase - arbitrary code execution

Comment 2

[Daniel Veditz [:dveditz]]

We wouldn't want to rush a fix for this into 1.8.1.8 at this point.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Potential fix

So, the problem is that when we create the XPCNativeWrapper for chrome, its parent comes from content, so we'll search in the content window's scope to see what prototype we should give it. Properties on the prototype are then able to override the real properties. I'm fixing this by not giving any XPCNativeWrappers any prototype at all and moving the one function on the current real prototype directly on the wrapper itself. Some possible side-effects would be that other Object.prototype properties will disappear (like __define[GS]etter__), but I'm not convinced that we need to expose those through XPCNativeWrapper.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 286119 [details] [diff] [review]
Potential fix

r=jst

Comment 5

[chris hofmann]

did this one land on the trunk, or is it waiting for post m9?

Comment 6

[Mike Schroepfer]

checkin needed?

Comment 7

[Mike Schroepfer]

Ping - Blake?  BKap?  Where are you... 

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Pong! Checked in.

Comment 9

[Mike Schroepfer]

(In reply to comment #8)
> Pong! Checked in.
> 

Sweet - thanks!

Comment 10

[moz_bug_r_a4]

Explicit XPCNativeWrappers still use a prototype that comes from content.

Comment 11

[moz_bug_r_a4]

Attachment: testcase 2

Actual: xxx
Expected: undefined

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: I could have sworn I did this before...

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Er, please ignore the parser/htmlparser stuff in that patch...

Comment 14

[Damon Sicore (:damons)]

Comment on attachment 291233 [details] [diff] [review]
I could have sworn I did this before...

re-request 1.9 approval once reviews are completed.  Thanks, bkap!

Comment 15

[Johnny Stenback (:jst)]

Comment on attachment 291233 [details] [diff] [review]
I could have sworn I did this before...

r+sr=jst for the XPCNativeWrapper.cpp changes, but the nsHTMLTokens.cpp changes are unrelated to this bug, right?

Comment 16

[Damon Sicore (:damons)]

Also, Blake, no approval request is actually needed to land this as it's a P1 beta 2 blocker.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Fixed again.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 1.8 branch patch

The only hand merging here was trivial (EnsureLegalActivity doesn't exist on the 1.8 branch and probably never should). This includes the patch for bug 407289 as well.

Comment 19

[Daniel Veditz [:dveditz]]

Comment on attachment 294508 [details] [diff] [review]
1.8 branch patch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 20

[Blake Kaplan (:mrbkap) (inactive)]

Fixed on the 1.8 branch.

Comment 21

[Al Billings [:abillings - ex-MoCo]]

Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.12) Gecko/2008012822 Firefox/2.0.0.12. Also verified testcase repro in 2.0.0.11.

Comment 22

[Al Billings [:abillings - ex-MoCo]]

Verified for trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008012804 Minefield/3.0b3pre.

Comment 23

[Alexander Sack]

Attachment: 1.8.0 patch (includes fix from cvs)

caillon, please sign off.

Comment 24

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 310222 [details] [diff] [review]
1.8.0 patch (includes fix from cvs)

a=caillon for 1.8.0.15

Comment 25

[Reed Loden [:reed]]

MOZILLA_1_8_0_BRANCH:

Checking in js/src/xpconnect/src/XPCNativeWrapper.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp,v  <--  XPCNativeWrapper.cpp
new revision: 1.31.2.4.2.7; previous revision: 1.31.2.4.2.6
done