336313 - Dynamic this binding is biting PAC

Status: VERIFIED FIXED

(Core :: Networking, defect, P1)

Description

[Blake Kaplan (:mrbkap) (inactive)]

I'm splitting this bug off of bug 321101 since there are two bugs reported there, and they're unrelated. This bug is about attachment 206496 [details], which I described as simple "highway robbery".

moz_bug_r_a4's clever testcase exploits the fact that when we call FindProxyForURL |this| is the nsProxyAutoConfig object itself and since there aren't any security checks once you're in JS-only land, the evil PAC script is able to trick the proxy auto config object into calling eval with its evil code (oh, how I hate seeing Components.stack in an alert ;-)).

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed minimal fix

This is the smallest fix I can come up with for this bug. It uses Function.prototype.call to force the |this| object to be the sandbox object, so doing evil things to the |this| object doesn't end up in privilege escalation.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

By the way, I also considered a patch to call this._sandBox.FindProxyForURL directly instead of storing the function in a member property; however, that would have changed behavior (if the PAC script did something weird), so I decided against it.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 220560 [details] [diff] [review]
Proposed minimal fix

sr=dveditz

approved for 1.8.0 branch, a=dveditz for drivers

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in everywhere.