Last Comment Bug 311403 - chrome XBL method.eval + onerror handler allows arbitary code execution
: chrome XBL method.eval + onerror handler allows arbitary code execution
Status: VERIFIED FIXED
[sg:critical] privilege escalation
: fixed1.8, verified1.7.13
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
:
: David Keeler [:keeler] (use needinfo?)
Mentors:
Depends on: 311952
Blocks: sbb? 311455
  Show dependency treegraph
 
Reported: 2005-10-06 14:03 PDT by shutdown
Modified: 2007-04-01 15:24 PDT (History)
6 users (show)
dveditz: blocking1.7.13+
dveditz: blocking‑aviary1.0.8+
dveditz: blocking1.8rc1+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.5+ only) (1.13 KB, text/html)
2005-10-06 14:11 PDT, shutdown
no flags Details
maybe wrong patch (1.21 KB, patch)
2005-10-06 17:27 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
better, maybe incomplete, patch (1.89 KB, patch)
2005-10-06 19:07 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
dveditz: approval‑aviary1.0.8+
dveditz: approval1.7.13+
Details | Diff | Splinter Review
fix script too (1.52 KB, patch)
2005-10-07 11:28 PDT, Blake Kaplan (:mrbkap)
brendan: review+
dveditz: approval‑aviary1.0.8+
dveditz: approval1.7.13+
asa: approval1.8rc1+
Details | Diff | Splinter Review
follow-on trunk cleanup patch (2.30 KB, patch)
2005-10-08 20:39 PDT, Brendan Eich [:brendan]
mrbkap: review+
dveditz: approval‑aviary1.0.8+
dveditz: approval1.7.13+
Details | Diff | Splinter Review
exploit testcase that works on 1.0.7 and suite 1.7.12 (236 bytes, text/html)
2005-10-13 08:57 PDT, moz_bug_r_a4
no flags Details

Description shutdown 2005-10-06 14:03:14 PDT
You can use onerror event handler to circumvent eval()'s principal checks
and can execute arbitary code with elevated privilege. See the testcase.
Comment 1 shutdown 2005-10-06 14:11:36 PDT
Created attachment 198728 [details]
testcase (1.5+ only)

Works on:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b5) Gecko/20051006 Firefox/1.4.1
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051006 Firefox/1.6a1

Firefox 1.0.x is not exploitable because onerror handler on 1.0.x
versions receive parameters different from exploitable versions.
Comment 2 Daniel Veditz [:dveditz] 2005-10-06 15:31:47 PDT
Confirming. Might be fixed by the patch for bug 311025 -- Blake's testing
Comment 3 Daniel Veditz [:dveditz] 2005-10-06 15:45:16 PDT
(In reply to comment #2)
> Might be fixed by the patch for bug 311025

It isn't.

Comment 4 Blake Kaplan (:mrbkap) 2005-10-06 17:27:13 PDT
Created attachment 198747 [details] [diff] [review]
maybe wrong patch

So the basic issue here is that when we compile the outer eval, we don't have a
scripted caller, so we compile it with NULL principals. From then on, the
scripted caller of the second eval also has no principals, so any security
checks that we would do succeed trivially. This patch gives the outer eval the
principals of the |this| object, which in the case of onerror is the content
window.
Comment 5 Blake Kaplan (:mrbkap) 2005-10-06 18:24:17 PDT
Comment on attachment 198747 [details] [diff] [review]
maybe wrong patch

better patch coming.
Comment 6 Blake Kaplan (:mrbkap) 2005-10-06 19:07:49 PDT
Created attachment 198762 [details] [diff] [review]
better, maybe incomplete, patch

This has r=brendan in person. There are more places that need to be patched to
deal correctly with null principals.
Comment 7 Blake Kaplan (:mrbkap) 2005-10-07 10:47:19 PDT
I forgot to mention, I checked this in yesterday. I'll come up with a more
complete patch soon.
Comment 8 Blake Kaplan (:mrbkap) 2005-10-07 11:28:42 PDT
Created attachment 198818 [details] [diff] [review]
fix script too

If it was possible to get one's hands on a new script object, compiled without
any principals, this refuses to execute it.

Note that this is a diff -w.
Comment 9 Daniel Veditz [:dveditz] 2005-10-08 15:37:12 PDT
Comment on attachment 198728 [details]
testcase (1.5+ only)

Note this testcase does not work in 1.0.x as mentioned in comment 1, but we
still want this principal mishandling fixed on the 1.0 branch because there may
be other ways to force the same issue.
Comment 10 Brendan Eich [:brendan] 2005-10-08 20:39:12 PDT
Created attachment 198977 [details] [diff] [review]
follow-on trunk cleanup patch

This should get into the 1.8 branch along with substantive fixes, so we can
diff trunk and branch without noise.

/be
Comment 11 Brendan Eich [:brendan] 2005-10-08 20:41:22 PDT
Comment on attachment 198818 [details] [diff] [review]
fix script too

Yet another rigtheous fix.  We should not have tolerated null principals in a
setting (rt->findObjectPrincipals onn-null, e.g.) where principals are needed
generally, this long.

/be
Comment 12 Blake Kaplan (:mrbkap) 2005-10-08 20:41:25 PDT
Comment on attachment 198977 [details] [diff] [review]
follow-on trunk cleanup patch

r=mrbkap
Comment 13 Blake Kaplan (:mrbkap) 2005-10-09 00:59:56 PDT
I checked in the additional patch. Marking this bug as FIXED.

Brendan, I wasn't sure if you wanted me to check your cleanup patch (attachment
198977 [details] [diff] [review]) in, so I left it alone.
Comment 14 Brendan Eich [:brendan] 2005-10-09 12:59:13 PDT
(In reply to comment #13)
> I checked in the additional patch. Marking this bug as FIXED.
> 
> Brendan, I wasn't sure if you wanted me to check your cleanup patch (attachment
> 198977 [edit]) in, so I left it alone.

I checked that into the trunk yesterday.  Just want to emphasize that we should
sync trunk and 1.8 branch when this goes in for rc1.

/be
Comment 15 moz_bug_r_a4 2005-10-13 08:55:45 PDT
(In reply to comment #9)
> Note this testcase does not work in 1.0.x as mentioned in comment 1, but we
> still want this principal mishandling fixed on the 1.0 branch because there may
> be other ways to force the same issue.

setTimeout can be used to force the same issue on 1.0.x.

setTimeout(eval, 0, code, obj);
When eval is called from timeout, scripted caller is null.
Comment 16 moz_bug_r_a4 2005-10-13 08:57:33 PDT
Created attachment 199419 [details]
exploit testcase that works on 1.0.7 and suite 1.7.12
Comment 17 shutdown 2005-10-13 11:11:56 PDT
(In reply to comment #15)
> setTimeout can be used to force the same issue on 1.0.x.

that is bug 311455.
Comment 18 Blake Kaplan (:mrbkap) 2005-10-14 18:00:09 PDT
Checked in on MOZILLA_1_8_BRANCH.
Comment 19 Daniel Veditz [:dveditz] 2006-02-06 12:30:04 PST
Comment on attachment 198818 [details] [diff] [review]
fix script too

aviary101/moz17 landing approval: a=dveditz for drivers. Please add the fixed1.7.13 and fixed-aviary1.0.8 keywords when landed.
Comment 20 Blake Kaplan (:mrbkap) 2006-02-09 17:35:28 PST
This should be fixed on the 1.7 branches from some earlier commits I did today.
Comment 21 Tracy Walker [:tracy] 2006-02-15 12:52:22 PST
verified with:
Windows:
Moz - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060215
Fx - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060215
Firefox/1.0.8
Macintosh:
Moz - Mozilla/5.0 (Macintosh; U;PPC Mac OS X Mach-O; en-US; rv:1.7.13)
Gecko/20060215 Firefox/1.0.8
Fx - Mozilla/5.0 (Macintosh; U;PPC Mac OS X Mach-O; en-US; rv:1.7.13)
Gecko/20060215 Firefox/1.0.8
Linux
Moz - Mozilla/5.0 (X11; U;Linux i686; en-US; rv:1.7.13) Gecko/20060215

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