337389 - PAC privilege escalation using Function.prototype.call

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[moz_bug_r_a4]

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]

Attachment: testcase - PAC script

Comment 2

[moz_bug_r_a4]

Attachment: testcase - html

Comment 3

[moz_bug_r_a4]

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) (inactive)]

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]

The testcases I've attached can also exploit this._sandBox.FindProxyForURL.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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]

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) (inactive)]

(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) (inactive)]

Attachment: 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]]

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) (inactive)]

Fix checked into trunk. Awaiting the workaround on moz_bug's size ;-).

Comment 12

[Darin Fisher]

> 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]

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]

Attachment: testcase 2 - PAC script using myIpAddress.watch.eval

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Hah! That was coming, I suppose. Okay, back to the other options...

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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 emeritus)]

Comment on attachment 224729 [details] [diff] [review]
Don't expose privileged functions

r=shaver

Comment 18

[Darin Fisher]

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) (inactive)]

Real fix checked in.

Comment 20

[Daniel Veditz [:dveditz]]

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) (inactive)]

Fix checked into the 1.8 branches.

Comment 22

[Alexander Sack]

Attachment: 1.0.x backport

Comment 23

[Bob Clary [:bc] (inactive)]

moz_bug_r_a4 verified this is fixed in 1.8 and 1.9 in Bug 321101 Comment #38 

Comment 24

[Brian Crowder]

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]

Ignore:  wrong bug.