PAC privilege escalation using Function.prototype.call

VERIFIED FIXED in mozilla1.9alpha1

Status

()

defect
P2
normal
VERIFIED FIXED
13 years ago
11 years ago

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

Tracking

({fixed1.8.0.5, verified1.8.1})

Trunk
mozilla1.9alpha1
x86
Windows XP
Points:
---
Bug Flags:
blocking1.7.14 ?
blocking-aviary1.0.9 ?
blocking1.9a1 +
blocking1.8.1 +
blocking1.8.0.5 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate] (critical if you use an untrusted PAC server))

Attachments

(7 attachments)

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.
Posted file testcase - html
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);
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.
The testcases I've attached can also exploit this._sandBox.FindProxyForURL.
Posted patch IndeedSplinter Review
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.
Attachment #221550 - Flags: review?(darin)
Flags: blocking1.9a1+
Flags: blocking1.8.1+
Flags: blocking1.8.0.5+
Flags: blocking1.8.0.4?
Flags: blocking1.7.14?
Flags: blocking-aviary1.0.9?
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.
Attachment #221550 - Flags: review?(darin) → review+
Assignee: nobody → mrbkap
(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...
Flags: blocking1.8.0.4?
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...
Attachment #224641 - Flags: superreview?(dveditz)
Attachment #224641 - Flags: review?(darin)
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
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.
Attachment #224641 - Flags: superreview?(dveditz) → superreview+
Attachment #224641 - Flags: review?(darin) → review+
Fix checked into trunk. Awaiting the workaround on moz_bug's size ;-).
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
> The next step is to rewrite PAC in C++, though, so
> this is worth a try.

See bug 340578 :-/
There are other paths to the privileged eval via Function.prototype.method or
Object.prototype.method.

myIpAddress.toString.eval, myIpAddress.watch.eval, ...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hah! That was coming, I suppose. Okay, back to the other options...
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.
Attachment #224729 - Flags: superreview?(darin)
Attachment #224729 - Flags: review?(shaver)
Comment on attachment 224729 [details] [diff] [review]
Don't expose privileged functions

r=shaver
Attachment #224729 - Flags: review?(shaver) → review+
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 ;-)
Attachment #224729 - Flags: superreview?(darin) → superreview+
Real fix checked in.
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Attachment #224729 - Flags: approval1.8.0.5?
Attachment #224729 - Flags: approval-branch-1.8.1?(darin)
Attachment #224729 - Flags: approval-branch-1.8.1?(darin) → approval-branch-1.8.1+
Comment on attachment 224729 [details] [diff] [review]
Don't expose privileged functions

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #224729 - Flags: approval1.8.0.5? → approval1.8.0.5+
Fix checked into the 1.8 branches.
Whiteboard: [sg:moderate] (critical if you use an untrusted PAC server)
moz_bug_r_a4 verified this is fixed in 1.8 and 1.9 in Bug 321101 Comment #38 
Status: RESOLVED → VERIFIED
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)?
Ignore:  wrong bug.
Group: core-security
You need to log in before you can comment on or make changes to this bug.