The default bug view has changed. See this FAQ.

PAC privilege escalation using Function.prototype.call

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
Networking
P2
normal
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

Tracking

({fixed1.8.0.5, verified1.8.1})

Trunk
mozilla1.9alpha1
x86
Windows XP
fixed1.8.0.5, verified1.8.1
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)

(Reporter)

Description

11 years ago
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.
(Reporter)

Comment 1

11 years ago
Created attachment 221547 [details]
testcase - PAC script
(Reporter)

Comment 2

11 years ago
Created attachment 221548 [details]
testcase - html
(Reporter)

Comment 3

11 years ago
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);
(Assignee)

Comment 4

11 years ago
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.
(Reporter)

Comment 5

11 years ago
The testcases I've attached can also exploit this._sandBox.FindProxyForURL.
(Assignee)

Comment 6

11 years ago
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.
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 7

11 years ago
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
(Assignee)

Comment 8

11 years ago
(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?
(Assignee)

Comment 9

11 years ago
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...
Attachment #224641 - Flags: superreview?(dveditz)
Attachment #224641 - Flags: review?(darin)
(Assignee)

Updated

11 years ago
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+

Updated

11 years ago
Attachment #224641 - Flags: review?(darin) → review+
(Assignee)

Comment 11

11 years ago
Fix checked into trunk. Awaiting the workaround on moz_bug's size ;-).
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 12

11 years ago
> The next step is to rewrite PAC in C++, though, so
> this is worth a try.

See bug 340578 :-/
(Reporter)

Comment 13

11 years ago
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 → ---
(Reporter)

Comment 14

11 years ago
Created attachment 224674 [details]
testcase 2 - PAC script using myIpAddress.watch.eval
(Assignee)

Comment 15

11 years ago
Hah! That was coming, I suppose. Okay, back to the other options...
(Assignee)

Comment 16

11 years ago
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.
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 18

11 years ago
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+
(Assignee)

Comment 19

11 years ago
Real fix checked in.
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Attachment #224729 - Flags: approval1.8.0.5?
Attachment #224729 - Flags: approval-branch-1.8.1?(darin)

Updated

11 years ago
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+
(Assignee)

Comment 21

11 years ago
Fix checked into the 1.8 branches.
Keywords: fixed1.8.0.5, fixed1.8.1
Whiteboard: [sg:moderate] (critical if you use an untrusted PAC server)

Comment 22

11 years ago
Created attachment 232734 [details] [diff] [review]
1.0.x backport

Comment 23

11 years ago
moz_bug_r_a4 verified this is fixed in 1.8 and 1.9 in Bug 321101 Comment #38 
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1

Comment 24

10 years ago
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

10 years ago
Ignore:  wrong bug.
Group: core-security
You need to log in before you can comment on or make changes to this bug.