Closed Bug 499910 Opened 10 years ago Closed 10 years ago

Arbitrary code execution with bug 498897

Categories

(Core :: Security, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1
Tracking Status
status1.9.2 --- beta1-fixed
blocking1.9.1 --- .2+
status1.9.1 --- .2-fixed

People

(Reporter: moz_bug_r_a4, Assigned: peterv)

References

Details

(Keywords: verified1.9.1, Whiteboard: [sg:critical])

Attachments

(2 files, 1 obsolete file)

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.
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.
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.
Probably not actually blocking 1.9.1, but flagging anyway.
Flags: wanted1.9.1.x+
Flags: blocking1.9.2?
Flags: blocking1.9.1?
We really should move the link loading I guess.
Peter, do you mean you want this bug to cover something other than the patch you checked in for bug 498897 this morning?
Whiteboard: [sg:dupe 498897]
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).
Assignee: nobody → peterv
Status: NEW → ASSIGNED
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.
Attached patch v1 (obsolete) — Splinter Review
This seems to fix things.
Attached patch v1.1Splinter Review
I'll try to make a mochitest.
Attachment #384670 - Attachment is obsolete: true
Attachment #384674 - Flags: superreview?(bzbarsky)
Attachment #384674 - Flags: review?(bzbarsky)
Flags: blocking1.9.2?
Flags: blocking1.9.2+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
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?
(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?
> 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.
Attachment #384674 - Flags: superreview?(bzbarsky)
Attachment #384674 - Flags: superreview+
Attachment #384674 - Flags: review?(bzbarsky)
Attachment #384674 - Flags: review+
Flags: wanted1.9.0.x-
Flags: blocking1.9.1.1?
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Has been on trunk since July 3rd. Exploitable bug triggered by certain types of extensions (eg AdBlock), regression from 3.0.
Attachment #388211 - Flags: approval1.9.1.1?
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?
> Why are they different?

Never mind, missed the added #ifdef in the trunk version here
mrbkap says that we can wait until 1.9.1.2 if that's coming in the next few weeks, which it is!
blocking1.9.1: --- → .2+
Flags: blocking1.9.1.1? → blocking1.9.1.1-
Attachment #388211 - Flags: approval1.9.1.1? → approval1.9.1.1-
Comment on attachment 388211 [details] [diff] [review]
v1.1 (branch version)

Approved for 1.9.1.2. a=ss for release-drivers
Attachment #388211 - Flags: approval1.9.1.2+
Whiteboard: [sg:dupe 498897] → [sg:critical]
Depends on: 502730
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?
Flags: in-testsuite?
You just need to be able to send a Link: HTTP header.  This should be completely doable with mochitest, I'd think.
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).
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)
Keywords: verified1.9.1
Forgot to add that I used Adblock Plus version 1.0.2
This bug should be all/all or?
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)
Keywords: fixed1.9.2
Group: core-security
You need to log in before you can comment on or make changes to this bug.