374071 - PAC privilege escalation by abusing the fix for bug 369213

Status: RESOLVED FIXED

(Core :: Security, defect)

Comment 1

[moz_bug_r_a4]

Attachment: testcase

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

So, the minimal fix here would be to add a method on the sandbox to create bound functions (though I don't actually see a function on the JS API to do so). The problem is that the only safe thing we can do with anything that has been in contact with the sandbox is call it -- getting even .call off of it is an invitation for malicious code to abuse us.

Comment 3

[Daniel Veditz [:dveditz]]

Moving out per Blake

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

Okay, this should fix the rest of these pesky PAC bugs. The idea is to use the safe JSObject wrapper, which is written in C++ to do what's really quite difficult to do in JS. Because the wrappers created here always create a scripted frame, we don't have to worry about the regular tricks, and because there's no chrome JS to call fun.call, we don't have to worry about the tricks used here.

There's one compatibility niggle: I now don't cache the value of FindProxyForURL, opting to look it up each time we call the function. This is the easiest way to ensure that 'this' for FindProxyForURL is the sandbox object. This means that scripts that modify FindProxyForURL in FindProxyForURL will behave differently, but I hope that that's not too big a problem.

There is some additional work done per host lookup, but it's simply the cost of calling two extra JS functions (pure JS, through the wrapper), so it shouldn't matter.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

I had to back this out because it turned the tinderboxes orange: XPCSafeJSObjectWrapper depends on being able to access an nsIScriptSecurityManager, but xpcshell doesn't give it one, so it crashes.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in, again.

Comment 8

[moz_bug_r_a4]

104 function proxyAlert(msg) {
105     // Ensure that we have a string.
106     if (typeof msg != "string")
107         msg = new XPCSafeJSObjectWraper(msg).toString();

There is a typo: "XPCSafeJSObjectWraper"

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Indeed, sorry about that. I just checked in the obvious fix to trunk:

Checking in netwerk/base/src/nsProxyAutoConfig.js;
/cvsroot/mozilla/netwerk/base/src/nsProxyAutoConfig.js,v  <--  nsProxyAutoConfig.js
new revision: 1.50; previous revision: 1.49
done

Comment 10

[Daniel Veditz [:dveditz]]

Is this going to make 1.8.1.8?