384750 - Arbitrary code execution by polluting implicit XPCNativeWrapper (without using eval)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[moz_bug_r_a4]

Please see bug 369211.

Content can access implicit XPCNativeWrappers without using eval(), by abusing
tabbrowser.xml's |contentWindow| and |contentDocument| getter functions.

Comment 1

[moz_bug_r_a4]

Attachment: testcase

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

So, at this point, I think the easiest way to fix these is to ensure that the callee has access to the XPCNativeWrapper in the XPCNativeWrapper code itself. I'll come up with a POC patch tomorrow or Monday, but I was wondering if there were better ideas.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

This stops the testcase, since any activity on an implicit native wrapper by non-system code is disallowed.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch that starts

The last patch didn't allow the browser to start. I'm not sure why I didn't see that before, though. This allows access to implicit wrappers with no JS on the stack (which happens when we seal the XPCNativeWrapper prototype on scope creation).

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

I had to back this out (sigh), Ts doesn't work with it.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fixing Ts

The problem was that we were initializing classes with random JS on the stack. This fixes that problem by setting aside whatever JS is currently running while we initialize XPConnect classes. I don't think that there's a security problem with this, since we're controlling everything that happens during class initialization.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 9

[Damon Sicore (:damons)]

Let's see if we can get a test in our test suite for this.

Comment 10

[Daniel Veditz [:dveditz]]

Do we still want this in 1.8.1.8, or should we wait until bug 387390 is also fixed (circumventing this fix)? If we land this we need to include the regression fixes, right?

Comment 11

[Daniel Veditz [:dveditz]]

Given this patch can be circumvented we'll wait for the real fix in the next update

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 270663 [details] [diff] [review]
Fixing Ts

This patch applies as-is.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 270663 [details] [diff] [review]
Fixing Ts

approved for 1.8.1.13, a=dveditz for release-drivers

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Checked into the 1.8 branch.

Comment 15

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

I don't get an alert with this in Firefox 2.0.0.12 (windows) but I get the giant context menu that has shown up with these sorts of exploit test cases. In 2.0.0.13, I get no alert and a normal context menu. Was this partially fixed in 2.0.0.12 already?

Comment 16

[moz_bug_r_a4]

Attachment: modified testcase

This works on fx2.0.0.12 (and does not work on fx2.0.0.13).  The reason that
the old testcase does not work is described in bug 387390 comment #9.

Comment 17

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

I tested with the new test case and repro'd the issue in 2.0.0.12. It is fixed in 2.0.0.13 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/2008031114 Firefox/2.0.0.13). Marking verified.

Thanks for the new testcase.

Comment 18

[Alexander Sack]

Comment on attachment 270663 [details] [diff] [review]
Fixing Ts

applies cleanly on 1.8.0

Comment 19

[Boris Zbarsky [:bzbarsky]]

It looks like this caused bug 384750, which was never addressed on branch (or linked to this bug).  See the thread at http://groups.google.com/group/mozilla.dev.security/browse_frm/thread/200083a2d0cb1562#