Last Comment Bug 311892 - XSS: fixes for bug 311024 and bug 311619 can be circumvented by using window.__proto__
: XSS: fixes for bug 311024 and bug 311619 can be circumvented by using window....
Status: RESOLVED FIXED
[sg:high] xss (splitwindows)
: testcase, verified1.7.13, verified1.8
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows XP
: P2 normal (vote)
: mozilla1.8rc1
Assigned To: Blake Kaplan (:mrbkap)
:
:
Mentors:
: 311962 (view as bug list)
Depends on: splitwindows
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-10 04:53 PDT by moz_bug_r_a4
Modified: 2013-08-18 14:34 PDT (History)
9 users (show)
dveditz: blocking1.7.13+
dveditz: blocking‑aviary1.0.8+
dveditz: blocking1.8rc1+
choller: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase 1 - steal cookie - eval(code, window.__proto__) (587 bytes, text/html)
2005-10-10 04:56 PDT, moz_bug_r_a4
no flags Details
testcase 2 - steal cookie - script.exec(window.__proto__) (611 bytes, text/html)
2005-10-10 04:57 PDT, moz_bug_r_a4
no flags Details
iframe's content - eval(code, window.__proto__) (167 bytes, text/html)
2005-10-10 23:52 PDT, moz_bug_r_a4
no flags Details
testcase 3 - eval(code, window.__proto__) without data: url (450 bytes, text/html)
2005-10-10 23:56 PDT, moz_bug_r_a4
no flags Details
iframe's content - script.exec(window.__proto__) (190 bytes, text/html)
2005-10-10 23:59 PDT, moz_bug_r_a4
no flags Details
wrong file (3.32 KB, patch)
2005-10-11 12:06 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
possible fix (2.97 KB, patch)
2005-10-11 12:07 PDT, Blake Kaplan (:mrbkap)
brendan: review+
Details | Diff | Splinter Review
better fix mrbkap pointed out we want, and he's right! (1.10 KB, patch)
2005-10-11 23:02 PDT, Brendan Eich [:brendan]
mrbkap: review-
Details | Diff | Splinter Review
Bah (4.24 KB, patch)
2005-10-13 18:23 PDT, Blake Kaplan (:mrbkap)
brendan: review+
asa: approval1.8rc1+
Details | Diff | Splinter Review
test.*.com/tests/mozilla.org/security/311892.zip (2.81 KB, application/zip)
2005-12-26 18:01 PST, Bob Clary [:bc:]
no flags Details

Description moz_bug_r_a4 2005-10-10 04:53:45 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050916
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051010 Firefox/1.6a1

Please see Bug 311024, Bug 311619 and Bug 310664.

Exploit:
code in subframe:

    function a(w) {
      with (w) alert(location.href + "\ncookie: " + document.cookie);
    }
    var f = eval("(" + a.toSource() + ");", window.__proto__);

code in main:

    var x = frames[0].f;
    frames[0].location = "http://www.yahoo.com/";
    x(frames[0]);

I've tested on the hourly build 2005101002.

Reproducible: Always

Steps to Reproduce:
Comment 1 moz_bug_r_a4 2005-10-10 04:56:16 PDT
Created attachment 199057 [details]
testcase 1 - steal cookie - eval(code, window.__proto__)
Comment 2 moz_bug_r_a4 2005-10-10 04:57:30 PDT
Created attachment 199058 [details]
testcase 2 - steal cookie - script.exec(window.__proto__)
Comment 3 Blake Kaplan (:mrbkap) 2005-10-10 16:44:21 PDT
I'm looking into this. This might end up being a data: url problem (where the
data URL is exposing bad principals).
Comment 4 moz_bug_r_a4 2005-10-10 23:48:30 PDT
(In reply to comment #3)
> I'm looking into this. This might end up being a data: url problem (where the
> data URL is exposing bad principals).

Exploits work without data: url.
Comment 5 moz_bug_r_a4 2005-10-10 23:52:04 PDT
Created attachment 199141 [details]
iframe's content - eval(code, window.__proto__)
Comment 6 moz_bug_r_a4 2005-10-10 23:56:09 PDT
Created attachment 199143 [details]
testcase 3 - eval(code, window.__proto__) without data: url
Comment 7 Blake Kaplan (:mrbkap) 2005-10-10 23:59:31 PDT
Yeah, I should have updated this bug earlier. brendan and I tracked this down
earlier and it has nothing to do with data: URIs and everything to do with eval
not checking its passed-in scopeobj if called directly. I'll have a patch tomorrow.
Comment 8 moz_bug_r_a4 2005-10-10 23:59:40 PDT
Created attachment 199144 [details]
iframe's content - script.exec(window.__proto__)
Comment 9 Blake Kaplan (:mrbkap) 2005-10-11 00:08:05 PDT
*** Bug 311962 has been marked as a duplicate of this bug. ***
Comment 10 Blake Kaplan (:mrbkap) 2005-10-11 12:06:41 PDT
Created attachment 199183 [details] [diff] [review]
wrong file

This is basically the patch from yesterday, but I cleaned it up a bit.
Comment 11 Blake Kaplan (:mrbkap) 2005-10-11 12:07:11 PDT
Comment on attachment 199183 [details] [diff] [review]
wrong file

ffs, wrong file :(
Comment 12 Blake Kaplan (:mrbkap) 2005-10-11 12:07:41 PDT
Created attachment 199184 [details] [diff] [review]
possible fix
Comment 13 Brendan Eich [:brendan] 2005-10-11 17:58:05 PDT
Comment on attachment 199184 [details] [diff] [review]
possible fix


>             callerVarObj = caller->varobj;
>             if (obj != callerVarObj) {
>                 /* Set fp->varobj too, for the compiler. */
>                 caller->varobj = fp->varobj = obj;
>                 setCallerVarObj = JS_TRUE;
>             }

This should move down too, per our face-to-face discussion.

>+    if (caller) {
>+        callerScopeChain = caller->scopeChain;
>+        if (scopeobj != callerScopeChain) {
>+            ok = CheckEvalAccess(cx, scopeobj, caller->script->principals);
>+            if (!ok)
>+                goto out;
>+
>+            scopeobj = js_NewObject(cx, &js_WithClass, scopeobj,
>+                                    callerScopeChain);
>+            if (!scopeobj) {
>+                ok = JS_FALSE;
>+                goto out;
>+            }
>+
>+            /* Set fp->scopeChain too, for the compiler. */
>+            caller->scopeChain = fp->scopeChain = scopeobj;
>+            setCallerScopeChain = JS_TRUE;
>+        }
>+    }

How about a brief comment, talking about change of scope, in addition to
indirect call, requiring this?

/be
Comment 14 Brendan Eich [:brendan] 2005-10-11 23:02:17 PDT
Created attachment 199267 [details] [diff] [review]
better fix mrbkap pointed out we want, and he's right!

This ought to work.  Blake will test and check into the trunk.	Thanks.

/be
Comment 15 Blake Kaplan (:mrbkap) 2005-10-12 02:33:10 PDT
Comment on attachment 199267 [details] [diff] [review]
better fix mrbkap pointed out we want, and he's right!

This doesn't work. Of course the script has access to window.__proto__, it
wouldn't be able to call eval with that parameter otherwise. The problem is
that searching the parent chain of window.__proto__ finds the outer window.
More thought is needed.
Comment 16 Scott MacGregor 2005-10-12 08:56:52 PDT
Comment on attachment 199184 [details] [diff] [review]
possible fix

clearing approval flag for an obsolete patch
Comment 17 Brendan Eich [:brendan] 2005-10-12 09:33:55 PDT
Comment on attachment 199267 [details] [diff] [review]
better fix mrbkap pointed out we want, and he's right!

Also, caller could be null.

Perhaps this is close, though.	It's moving in the right direction to separate
o.eval(s) and eval(s, o) cases.

/be
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-12 17:35:25 PDT
Blake and I talked this over and it seems like we need to make sure we never end
up with objects whose parent is an outer window to be safe against attacks like
this. The DOM objects currently parented at an outer window are
window.__proto__, window.navigator (including prototypes), and window.location
(also including prototypes). So this is getting rather messy, lots of things to
tune by hand to get all this right. Anyone have any ideas on a better approach here?
Comment 19 Brendan Eich [:brendan] 2005-10-12 18:33:33 PDT
(In reply to comment #18)
> Anyone have any ideas on a better approach here?

No :-/.  I told blake to fix it this way originally (I warned him, but did he
listen?  Nooooooooooooo!  ;-)

Sucks.  I'll help.

/be

Comment 20 Blake Kaplan (:mrbkap) 2005-10-13 00:07:53 PDT
We need to make sure not to regress bug 304284 with these changes.
Comment 21 Blake Kaplan (:mrbkap) 2005-10-13 18:23:02 PDT
Created attachment 199494 [details] [diff] [review]
Bah

Brendan caught me when I was hungry and suggested this. This places an
arbitrary limitation on the scope object passed to either eval() or
script.exec() that it not have an innerizable object on its proto chain. The
comment is too brief since I don't want to give too much away in it (though I'm
not sure how much we have to worry about this).
Comment 22 Brendan Eich [:brendan] 2005-10-13 18:39:45 PDT
Comment on attachment 199494 [details] [diff] [review]
Bah

Nits: ScopeChain, not ScopeObject, and maybe common the "Script.prototype.exec"
strict into a static const char [] explicitly?

/be
Comment 23 Blake Kaplan (:mrbkap) 2005-10-14 12:06:05 PDT
Fix checked into trunk.
Comment 24 Blake Kaplan (:mrbkap) 2005-10-14 12:06:47 PDT
Comment on attachment 199494 [details] [diff] [review]
Bah

This patch fixes a security bug by disallowing bad scoped objects from being
passed into eval.
Comment 25 Blake Kaplan (:mrbkap) 2005-10-14 14:01:59 PDT
Comment on attachment 199494 [details] [diff] [review]
Bah

Actually, I'm going to roll this into my monster "security fixer" branch patch.
Comment 26 Blake Kaplan (:mrbkap) 2005-10-14 17:59:40 PDT
Checked in on MOZILLA_1_8_BRANCH.
Comment 27 Bob Clary [:bc:] 2005-11-09 15:15:33 PST
tested with firefox 1.5 rc2 on winxp/linux
testcase 1: pass
testcase 2: pass
testcase 3: pass
Comment 28 Bob Clary [:bc:] 2005-12-26 18:01:22 PST
Created attachment 206881 [details]
test.*.com/tests/mozilla.org/security/311892.zip

moz_bug_r_a4's tests.
Comment 29 Daniel Veditz [:dveditz] 2006-02-06 19:29:23 PST
Fixed on the aviary1.0/mozilla1.7 branches by the split-window alternative (bug 316589)
Comment 30 Jay Patel [:jay] 2006-02-15 13:37:21 PST
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060215 Firefox/1.0.8, all 3 testcases pass.
Comment 31 Bob Clary [:bc:] 2006-02-22 02:44:09 PST
v ff on 1.7.13 windows/mac 20060221 builds, moz on 1.7.13 windows 20060221

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