Last Comment Bug 337389 - PAC privilege escalation using Function.prototype.call
: PAC privilege escalation using Function.prototype.call
Status: VERIFIED FIXED
[sg:moderate] (critical if you use an...
: fixed1.8.0.5, verified1.8.1
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: x86 Windows XP
: P2 normal (vote)
: mozilla1.9alpha1
Assigned To: Blake Kaplan (:mrbkap)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-09 23:22 PDT by moz_bug_r_a4
Modified: 2008-10-10 15:22 PDT (History)
9 users (show)
dveditz: blocking1.7.14?
dveditz: blocking‑aviary1.0.9?
dveditz: blocking1.9a1+
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase - PAC script (136 bytes, text/plain)
2006-05-09 23:24 PDT, moz_bug_r_a4
no flags Details
testcase - html (579 bytes, text/html)
2006-05-09 23:25 PDT, moz_bug_r_a4
no flags Details
Indeed (2.58 KB, patch)
2006-05-10 00:14 PDT, Blake Kaplan (:mrbkap)
darin.moz: review+
Details | Diff | Splinter Review
Hacky, yet speedy alternative (1.40 KB, patch)
2006-06-06 18:21 PDT, Blake Kaplan (:mrbkap)
darin.moz: review+
dveditz: superreview+
Details | Diff | Splinter Review
testcase 2 - PAC script using myIpAddress.watch.eval (202 bytes, text/plain)
2006-06-07 02:25 PDT, moz_bug_r_a4
no flags Details
Don't expose privileged functions (7.82 KB, patch)
2006-06-07 11:45 PDT, Blake Kaplan (:mrbkap)
shaver: review+
darin.moz: superreview+
darin.moz: approval‑branch‑1.8.1+
dveditz: approval1.8.0.5+
Details | Diff | Splinter Review
1.0.x backport (6.35 KB, patch)
2006-08-08 08:24 PDT, Alexander Sack
no flags Details | Diff | Splinter Review

Description moz_bug_r_a4 2006-05-09 23:22:14 PDT
It is possible to run arbitrary code with chrome privileges by combining a PAC
and an html.

This exploit uses the fact that the following code works as |eval("code")|.

  var obj = {
    valueOf : function() { return eval; }
  };
  Function.prototype.call.call(obj, "", "code");

PAC script can make sandbox.valueOf() return a privileged eval (e.g.
myIpAddress.eval), and assign Function.prototype.call to FindProxyForURL.

  this.valueOf = function() { return myIpAddress.eval; };
  this.FindProxyForURL = Function.prototype.call;

When attempting to load a url "http://x(y)/", the arguments that are passed to
FindProxyForURL are: testURI = "http://x(y)/", testHost = "x(y)".  Thus,
testHost can be used as a JS code.  (testURI is also valid JS code, but it's
harmless, since http: is a label and //x(y)/ is a single line comment.) 


Steps to Reproduce:
1. Setup a PAC file testcase on web server, or save it to local hard disk.
2. Set Automatic proxy configuration URL to its URL.
3. Open an html testcase.

An alert dialog that shows Components.stack will appear.
Comment 1 moz_bug_r_a4 2006-05-09 23:24:27 PDT
Created attachment 221547 [details]
testcase - PAC script
Comment 2 moz_bug_r_a4 2006-05-09 23:25:51 PDT
Created attachment 221548 [details]
testcase - html
Comment 3 moz_bug_r_a4 2006-05-09 23:27:44 PDT
I think this bug could be fixed by using evalInSandbox to call FindProxyForURL.

var s = "FindProxyForURL(" + uneval(testURI) + "," + uneval(testHost) + ");";
return Components.utils.evalInSandbox(s, this._sandBox);
Comment 4 Blake Kaplan (:mrbkap) 2006-05-09 23:41:36 PDT
Hmm, I think a simpler solution would be to just use:
this._sandBox.FindProxyForURL(testURI, testHost);

to avoid the decompilation/recompilation. Is there a way to attack that? What other advantages that I'm not seeing do we gain by going back through evalInSandbox?

I chose not to go this way last time since it changes behavior when the PAC script sets FindProxyForURL on subsequent calls to it, but now I don't think that's too bad.
Comment 5 moz_bug_r_a4 2006-05-10 00:03:27 PDT
The testcases I've attached can also exploit this._sandBox.FindProxyForURL.
Comment 6 Blake Kaplan (:mrbkap) 2006-05-10 00:14:02 PDT
Created attachment 221550 [details] [diff] [review]
Indeed

Sorry, I wasn't thinking all the way through the exploit. This is obviously the best solution, if not the most performant, all code execution happens inside the sandbox, so any hack attempts bounce off of the walls, as they were.
Comment 7 Darin Fisher 2006-05-11 13:55:49 PDT
Comment on attachment 221550 [details] [diff] [review]
Indeed

OK.. how bad is this from a perf point of view?  Keep in mind that this code will be executed once for every single URI that is loaded by the browser.
Comment 8 Blake Kaplan (:mrbkap) 2006-05-16 15:01:14 PDT
(In reply to comment #7)
> (From update of attachment 221550 [details] [diff] [review] [edit])
> OK.. how bad is this from a perf point of view?  Keep in mind that this code
> will be executed once for every single URI that is loaded by the browser.
> 

This is fairly thrashy because it creates a new context for every call. I'll try to fix that first... I'm not really sure what to do about this bug in the meantime. The effects might even be tolerable as is...
Comment 9 Blake Kaplan (:mrbkap) 2006-06-06 18:21:21 PDT
Created attachment 224641 [details] [diff] [review]
Hacky, yet speedy alternative

This isn't very robust, nor is it very general, but it'll fix the testcase in this bug without seriously regressing netwerk performance over PAC...
Comment 10 Daniel Veditz [:dveditz] 2006-06-06 18:46:29 PDT
Comment on attachment 224641 [details] [diff] [review]
Hacky, yet speedy alternative

sr=dveditz

I have a sinking feeling moz_bug_r_a4 will quickly bypass this as well, but at the moment I can't see how. The next step is to rewrite PAC in C++, though, so this is worth a try.
Comment 11 Blake Kaplan (:mrbkap) 2006-06-06 18:59:21 PDT
Fix checked into trunk. Awaiting the workaround on moz_bug's size ;-).
Comment 12 Darin Fisher 2006-06-06 19:08:24 PDT
> The next step is to rewrite PAC in C++, though, so
> this is worth a try.

See bug 340578 :-/
Comment 13 moz_bug_r_a4 2006-06-07 02:21:12 PDT
There are other paths to the privileged eval via Function.prototype.method or
Object.prototype.method.

myIpAddress.toString.eval, myIpAddress.watch.eval, ...
Comment 14 moz_bug_r_a4 2006-06-07 02:25:29 PDT
Created attachment 224674 [details]
testcase 2 - PAC script using myIpAddress.watch.eval
Comment 15 Blake Kaplan (:mrbkap) 2006-06-07 09:06:44 PDT
Hah! That was coming, I suppose. Okay, back to the other options...
Comment 16 Blake Kaplan (:mrbkap) 2006-06-07 11:45:28 PDT
Created attachment 224729 [details] [diff] [review]
Don't expose privileged functions

This brings back sandbox.importFunction. With this patch, there are no more chrome functions in the sandbox for malicious scripts to pull eval off of. This method won't work for GreaseMonkey, but since GreaseMonkey doesn't try to call functions pulled out of the sandbox, I think it can get away without using this sort of hack.
Comment 17 Mike Shaver (:shaver -- probably not reading bugmail closely) 2006-06-07 12:14:41 PDT
Comment on attachment 224729 [details] [diff] [review]
Don't expose privileged functions

r=shaver
Comment 18 Darin Fisher 2006-06-07 12:45:42 PDT
Comment on attachment 224729 [details] [diff] [review]
Don't expose privileged functions

rs=me on the js+xpc parts, sr=me on the necko parts ;-)
Comment 19 Blake Kaplan (:mrbkap) 2006-06-07 13:22:11 PDT
Real fix checked in.
Comment 20 Daniel Veditz [:dveditz] 2006-06-15 14:26:42 PDT
Comment on attachment 224729 [details] [diff] [review]
Don't expose privileged functions

approved for 1.8.0 branch, a=dveditz for drivers
Comment 21 Blake Kaplan (:mrbkap) 2006-06-15 18:43:11 PDT
Fix checked into the 1.8 branches.
Comment 22 Alexander Sack 2006-08-08 08:24:55 PDT
Created attachment 232734 [details] [diff] [review]
1.0.x backport
Comment 23 Bob Clary [:bc:] 2006-08-23 07:15:38 PDT
moz_bug_r_a4 verified this is fixed in 1.8 and 1.9 in Bug 321101 Comment #38 
Comment 24 Brian Crowder 2007-01-16 11:53:17 PST
Does the patch in bug 314874 fix this (I think this depends on our "extension"/broken interpretation of executing "valueOf" on things that are already objects, in call and apply)?
Comment 25 Brian Crowder 2007-01-16 11:53:51 PST
Ignore:  wrong bug.

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