Closed
Bug 311403
Opened 19 years ago
Closed 19 years ago
chrome XBL method.eval + onerror handler allows arbitary code execution
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
VERIFIED
FIXED
People
(Reporter: sync2d, Assigned: mrbkap)
References
Details
(Keywords: fixed1.8, verified1.7.13, Whiteboard: [sg:critical] privilege escalation)
Attachments
(5 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
mrbkap
:
review+
dveditz
:
approval-aviary1.0.8+
dveditz
:
approval1.7.13+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
brendan
:
review+
dveditz
:
approval-aviary1.0.8+
dveditz
:
approval1.7.13+
asa
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
dveditz
:
approval-aviary1.0.8+
dveditz
:
approval1.7.13+
|
Details | Diff | Splinter Review |
(deleted),
text/html
|
Details |
You can use onerror event handler to circumvent eval()'s principal checks
and can execute arbitary code with elevated privilege. See the testcase.
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•19 years ago
|
||
Confirming. Might be fixed by the patch for bug 311025 -- Blake's testing
Blocks: sbb?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8rc1+
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8+
Whiteboard: [sg:high] privilege escalation
Updated•19 years ago
|
Whiteboard: [sg:high] privilege escalation → [sg:critical] privilege escalation
Comment 3•19 years ago
|
||
(In reply to comment #2)
> Might be fixed by the patch for bug 311025
It isn't.
Assignee | ||
Comment 4•19 years ago
|
||
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.
Attachment #198747 -
Flags: review?(brendan)
Assignee | ||
Comment 5•19 years ago
|
||
Comment on attachment 198747 [details] [diff] [review]
maybe wrong patch
better patch coming.
Attachment #198747 -
Attachment is obsolete: true
Attachment #198747 -
Flags: review?(brendan)
Assignee | ||
Comment 6•19 years ago
|
||
This has r=brendan in person. There are more places that need to be patched to
deal correctly with null principals.
Assignee | ||
Comment 7•19 years ago
|
||
I forgot to mention, I checked this in yesterday. I'll come up with a more
complete patch soon.
Assignee | ||
Comment 8•19 years ago
|
||
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.
Attachment #198818 -
Flags: review?(brendan)
Comment 9•19 years ago
|
||
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.
Attachment #198728 -
Attachment description: testcase → testcase (1.5+ only)
Comment 10•19 years ago
|
||
This should get into the 1.8 branch along with substantive fixes, so we can
diff trunk and branch without noise.
/be
Attachment #198977 -
Flags: review?(mrbkap)
Comment 11•19 years ago
|
||
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
Attachment #198818 -
Flags: review?(brendan)
Attachment #198818 -
Flags: review+
Attachment #198818 -
Flags: approval1.8rc1?
Assignee | ||
Comment 12•19 years ago
|
||
Comment on attachment 198977 [details] [diff] [review]
follow-on trunk cleanup patch
r=mrbkap
Attachment #198977 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 13•19 years ago
|
||
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.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 14•19 years ago
|
||
(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
Updated•19 years ago
|
Attachment #198818 -
Flags: approval1.8rc1? → approval1.8rc1+
Comment 15•19 years ago
|
||
(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•19 years ago
|
||
Reporter | ||
Comment 17•19 years ago
|
||
(In reply to comment #15)
> setTimeout can be used to force the same issue on 1.0.x.
that is bug 311455.
Updated•19 years ago
|
Flags: testcase+
Updated•19 years ago
|
Attachment #198762 -
Flags: approval1.7.13+
Attachment #198762 -
Flags: approval-aviary1.0.8+
Comment 19•19 years ago
|
||
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.
Attachment #198818 -
Flags: approval1.7.13+
Attachment #198818 -
Flags: approval-aviary1.0.8+
Updated•19 years ago
|
Attachment #198977 -
Flags: approval1.7.13+
Attachment #198977 -
Flags: approval-aviary1.0.8+
Assignee | ||
Comment 20•19 years ago
|
||
This should be fixed on the 1.7 branches from some earlier commits I did today.
Keywords: fixed-aviary1.0.8,
fixed1.7.13
Updated•19 years ago
|
Status: RESOLVED → VERIFIED
Comment 21•19 years ago
|
||
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
Updated•18 years ago
|
Group: security
Updated•18 years ago
|
Flags: in-testsuite+ → in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•