499910 - Arbitrary code execution with bug 498897

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[moz_bug_r_a4]

I saw bug 498897 when that was public, and now I cannot see that bug.

With the fix (http://hg.mozilla.org/mozilla-central/rev/dca035c2bb32), it is
still exploitable.  The document in question and nodes in that document are
system objects (JS_IsSystemObject() returns true), thus XPC*Wrapper auto
wrapping in XPCConvert::NativeInterface2JSObject() does not work.

I have another testcase that works without the fix for bug 498897.  I'll attach
it if needed.

Comment 2

[moz_bug_r_a4]

Now, I can see bug 498897, and there is no exploit testcase in that bug.  I'll
attach another testcase to demonstrate that bug 498897 is really exploitable.

Comment 3

[moz_bug_r_a4]

Attachment: testcase 2 - cgi script

Without the fix for bug 498897, a content XBL <constructor> can run with the
chrome privileges.

This works on fx-3.5pre-2009-06-22-04 with Adblock Plus 1.0.2.

Steps to reproduce:
1. Install Adblock Plus.
2. load this testcase.

An exploit alert will appear.

Comment 4

[Samuel Sidler (old account; do not CC)]

Probably not actually blocking 1.9.1, but flagging anyway.

Comment 5

[Peter Van der Beken [:peterv]]

We really should move the link loading I guess.

Comment 6

[Daniel Veditz [:dveditz]]

Peter, do you mean you want this bug to cover something other than the patch you checked in for bug 498897 this morning?

Comment 7

[Peter Van der Beken [:peterv]]

Boris did suggest the link loading in bug 498897 comment 30, but discussing with mrbkap we thought the reparenting would be enough (it did fix the testcase in that bug). So I guess this bug should cover fixing the remaining security issue (and then we should probably back out the patch from bug 498897 and add an assertion instead).

Comment 8

[Peter Van der Beken [:peterv]]

You can dupe to the other bug if you want, I don't really care. As long as there's a bug somewhere to attach the patch to.

Comment 9

[Peter Van der Beken [:peterv]]

Attachment: v1

This seems to fix things.

Comment 10

[Peter Van der Beken [:peterv]]

Attachment: v1.1

I'll try to make a mochitest.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Don't we need to revoke the event in our destructor?  Or are we guaranteed that WillBuildModel will be called?

For that matter, why bother with the event at all?  Why not just do the call from WillBuildModel?

Can you file a followup bug on the fact that we screw up the case when HTTP has multiple Link headers, as far as I can see?

Comment 12

[Peter Van der Beken [:peterv]]

(In reply to comment #11)
> Don't we need to revoke the event in our destructor?  Or are we guaranteed that
> WillBuildModel will be called?

nsRevocableEventPtr revokes the event from its destructor.

> For that matter, why bother with the event at all?  Why not just do the call
> from WillBuildModel?

Seems like we want to start processing the link header (also doing DNS and other prefetching) as soon as possible, no? Waiting for the first data could delay it a bit.

> Can you file a followup bug on the fact that we screw up the case when HTTP has
> multiple Link headers, as far as I can see?

You'll need to say more about this, I'm not sure what is going wrong?

Comment 13

[Boris Zbarsky [:bzbarsky]]

> You'll need to say more about this, I'm not sure what is going wrong?

Oh, nevermind.  I missed the ProcessLink calls in the loop.

The rest of comment 12 makes sense.

Comment 14

[Peter Van der Beken [:peterv]]

Attachment: v1.1 (branch version)

Has been on trunk since July 3rd. Exploitable bug triggered by certain types of extensions (eg AdBlock), regression from 3.0.

Comment 15

[Daniel Veditz [:dveditz]]

The branch version sort of includes the patch from bug bug 498897, but only a #ifdef DEBUG check whereas the trunk tries to reparent the wrapper (in nsDocument::SetScriptGlobalObject). Why are they different? Have you thought better of reparenting and there should be a trunk patch to make that debug-only too?

Comment 16

[Daniel Veditz [:dveditz]]

> Why are they different?

Never mind, missed the added #ifdef in the trunk version here

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

mrbkap says that we can wait until 1.9.1.2 if that's coming in the next few weeks, which it is!

Comment 18

[Samuel Sidler (old account; do not CC)]

Comment on attachment 388211 [details] [diff] [review]
v1.1 (branch version)

Approved for 1.9.1.2. a=ss for release-drivers

Comment 19

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/774349c15597

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Is there a way to test the fix without the need to have a web server with cgi support? Would an automated test be possible?

Comment 21

[Boris Zbarsky [:bzbarsky]]

You just need to be able to send a Link: HTTP header.  This should be completely doable with mochitest, I'd think.

Comment 22

[Peter Van der Beken [:peterv]]

I've tried making one, but detecting the difference in wrapper parent in the XBL constructor is not trivial (because the content policy is embedded in the file).

Comment 23

[juan becerra [:juanb]]

Verified using test cases in comment #1 and comment #3. On 3.5 the first test case did not open an alert, but an exception was shown in the error console. The second test case did open an alert.

On 3.5.2 the first case does not open an alert nor does it show an exception in the error console. The second test case does not open an alert, and it shows the following in the error console: 

Error: Permission denied for <http://localhost> to get property XPCComponents.stack
Source File: http://localhost/cgi-bin/exec-link-1-xbl.cgi
Line: 17

Using:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729)

Comment 24

[juan becerra [:juanb]]

Forgot to add that I used Adblock Plus version 1.0.2

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This bug should be all/all or?

Comment 26

[Mike Beltzner [:beltzner, not reading bugmail]]

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)