Last Comment Bug 461743 - A chrome function runs on content can cause arbitrary code execution hole
: A chrome function runs on content can cause arbitrary code execution hole
Status: VERIFIED FIXED
: [sg:critical][fixed in bug 464620]
: verified1.8.1.19, verified1.9.0.5, verified1.9.1
Product: Core
Classification: Components
Component: Security
: unspecified
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Simon Bünzli
: toolkit
:
:
:
  Show dependency treegraph
 
Reported: 2008-10-27 00:45 PDT by moz_bug_r_a4
Modified: 2010-02-13 12:21 PST (History)
19 users (show)
benjamin: blocking1.9.1+
samuel.sidler+old: blocking1.9.0.5+
samuel.sidler+old: wanted1.9.0.x+
samuel.sidler+old: blocking1.8.1.19+
samuel.sidler+old: wanted1.8.1.x+
asac: wanted1.8.0.x-
samuel.sidler+old: in‑testsuite?
See Also:
Crash Signature:


Attachments
use the chrome window's setTimeout (1.59 KB, patch)
2008-10-27 12:40 PDT, Simon Bünzli
mrbkap: review+
Details | Diff | Splinter Review
updated to comment #7 and comment #8 (1.57 KB, patch)
2008-10-27 13:31 PDT, Simon Bünzli
dveditz: approval1.8.1.19+
dveditz: approval1.9.0.5+
Details | Diff | Splinter Review
tests for bug 459906 and this bug (10.04 KB, patch)
2008-10-28 17:12 PDT, Simon Bünzli
mrbkap: review+
Details | Diff | Splinter Review
branch patch (1.65 KB, patch)
2008-11-13 11:42 PST, Simon Bünzli
no flags Details | Diff | Splinter Review

Summon comment box

Description moz_bug_r_a4 2008-10-27 00:45:37 PDT
A chrome function executed by content.setTimeout(chromeFun, ...) or
content.addEvenetListener(type, chromeFun, ...) runs on the content JS context.
Thus, if the chrome function modifies the content DOM, and a content-registered
mutation event listener is called, the listener's scripted caller is the chrome
function.  This is a similar problem to bug 360529.
Comment 1 moz_bug_r_a4 2008-10-27 00:46:45 PDT
There is an exploitable code in nsSessionStore.js.

http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/nsSessionStore.js#1927

1927       if (aData.innerHTML) {
1928         aContent.setTimeout(
1929               function(aHTML) {
1930                 if (aContent.document.designMode == "on") {
1931                   aContent.document.body.innerHTML = aHTML;
1932                 }
1933               }, 0, aData.innerHTML);
1934       }
Comment 2 moz_bug_r_a4 2008-10-27 00:48:07 PDT
Created attachment 344853 [details]
testcase 1

This works in trunk/fx3 on Windows/Linux, and in fx2 on Windows.
Comment 3 moz_bug_r_a4 2008-10-27 00:49:09 PDT
Created attachment 344854 [details]
testcase 2

This works in fx2 on Windows/Linux.
Comment 4 Johnathan Nightingale [:johnath] 2008-10-27 05:53:51 PDT
testcase 1, unsurprisingly, works on mac trunk as well.
Comment 5 Samuel Sidler (old account; do not CC) 2008-10-27 11:47:23 PDT
Blake, can you look at this for 1.8.1.19/1.9.0.5
Comment 6 Simon Bünzli 2008-10-27 12:40:18 PDT
Created attachment 344952 [details] [review]
use the chrome window's setTimeout

In this case, sanitizing innerHTML wouldn't have helped at all.
Comment 7 Simon Bünzli 2008-10-27 12:42:30 PDT
Comment on attachment 344952 [details] [review]
use the chrome window's setTimeout

BTW: |this| is a <xul:browser>, so correctly we'd even rather use

> let window = this.ownerDocument.defaultView;
Comment 8 Blake Kaplan (:mrbkap) 2008-10-27 13:24:44 PDT
Comment on attachment 344952 [details] [review]
use the chrome window's setTimeout

>diff -r 1622ca12ba25 browser/components/sessionstore/src/nsSessionStore.js
>+        window.setTimeout(function(aHTML) {
>+          if (aContent.document.designMode == "on")
>+            aContent.document.body.innerHTML = aData.innerHTML;
>+        }, 0);

You don't need to take an aHTML parameter anymore.

r=mrbkap with that. Thanks.
Comment 9 Simon Bünzli 2008-10-27 13:31:49 PDT
Created attachment 344964 [details] [review]
updated to comment #7 and comment #8
Comment 10 Simon Bünzli 2008-10-27 13:35:12 PDT
Does bug 460882 cover the actual core issue as well?
Comment 11 Samuel Sidler (old account; do not CC) 2008-10-27 13:40:00 PDT
Can you create an automated testcase (but not checkin until after we ship the fix)?
Comment 12 Simon Bünzli 2008-10-27 13:44:30 PDT
I'll look into writing tests for this bug and bug 459906 once I've got some time (probably next weekend).
Comment 13 Blake Kaplan (:mrbkap) 2008-10-27 13:46:36 PDT
(In reply to comment #10)
> Does bug 460882 cover the actual core issue as well?

No, I'll file a followup on it.
Comment 14 Blake Kaplan (:mrbkap) 2008-10-27 15:15:49 PDT
Actually, is it possible to use this event in combination with the exploit from bug 360529 to get around the proper use of XPCNativeWrappers? Sanitation might be the only way to fix that bug for real, if so...
Comment 15 Blake Kaplan (:mrbkap) 2008-10-27 16:14:41 PDT
(In reply to comment #13)
> No, I'll file a followup on it.

That's now bug 461861.
Comment 16 moz_bug_r_a4 2008-10-27 18:13:33 PDT
(In reply to comment #14)
> Actually, is it possible to use this event in combination with the exploit from
> bug 360529 to get around the proper use of XPCNativeWrappers?

Yes, the exploit for bug 360529 can use mutation events (see bug 360529 comment
#23 and bug 430658).
Comment 17 Blake Kaplan (:mrbkap) 2008-10-27 18:19:04 PDT
Whoops, I meant bug 459906.
Comment 18 Simon Bünzli 2008-10-28 17:12:50 PDT
Created attachment 345197 [details] [review]
tests for bug 459906 and this bug

I've adapted the testcases from these bugs for our browser tests. These fail both without the patches and pass with them.
Comment 19 Daniel Veditz 2008-11-03 11:21:24 PST
Comment on attachment 344964 [details] [review]
updated to comment #7 and comment #8

approved for 1.9.0.5 and 1.8.1.19, a=dveditz for release-drivers

(please watch tinderbox for the tree to open before landing)
Comment 20 Shawn Wilsher :sdwilsh 2008-11-13 11:06:03 PST
This patch does not apply cleanly to 1.9.0, and I don't know the code enough to make the change myself (code around this block is different, and I'm not sure if that changes the fix).
Comment 21 Simon Bünzli 2008-11-13 11:42:03 PST
Created attachment 348023 [details] [review]
branch patch

Shawn: Thanks for the check-in!
Comment 22 Shawn Wilsher :sdwilsh 2008-11-13 11:51:24 PST
Checking in browser/components/sessionstore/src/nsSessionStore.js;
new revision: 1.104; previous revision: 1.103

Not sure when this will be able to land on trunk, and I don't have a tree to land it on 1.8.1 (maybe mrbkap can do that since I think he has other patches to land there as well?).
Comment 23 Simon Bünzli 2008-11-19 08:16:33 PST
This bug was FIXED in bug 464620 where patches hadn't been checked in already. Tests will land in bug 464620 as well.
Comment 24 Al Billings [:abillings] 2008-11-25 16:20:09 PST
Verified for 1.8.1.19 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.19pre) Gecko/2008112503 BonEcho/2.0.0.19pre. 

Verified for 1.9.0.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008112505 GranParadiso/3.0.5pr.

Doesn't reproduce in current trunk. Marking as verified.

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