Opened 9 years ago

Closed 9 years ago

#2819 closed defect (fixed)

Fix JS Hooks in FF4 using new JS 1.8.5 features

Reported by: gk Owned by: mikeperry
Priority: Very High Milestone:
Component: Applications/Torbutton Version:
Severity: Keywords: MikePerryIteration20110417
Cc: Actual Points: 4
Parent ID: Points: 4
Reviewer: Sponsor:

Description

I have another one. It seems possible to remove at least some of Torbuttons JS hooks in FF4. I have just implemented code for innerWidth and innerHeight but other properties should be removable as well. See:

http://debian.anonymous-proxy-servers.net:81/Unhooking.html

Child Tickets

Change History (11)

comment:1 Changed 9 years ago by gk

Okay, I just read the comment in jshooks.js today and saw that the commented window.proto = null is causing the issues. Furthermore, I recognized that this problem was already uncovered by Greogory Fleischer some years ago. Nevertheless, I found a solution for fixing it. You have to split the hooks depending on the Firefox version (fun, I know). If the user has a FF3 you may set window.proto = null as usual. If you have a FF4 you do not do this but use the Object.defineProperty with configurable set to false. It's ES5 stuff available in FF4. The code would then be something like:

Object.defineProperty(window.proto, "innerWidth", {

get: function() { return Math.round(origWidth/50.0)*50;},
configurable: false});

in order to fake the innerWidth property. I just tested it and it seems to work. (And yes, an attacker is not able to revert this using things like Object.defineProperty(foo, "bar", { configurable: true}).)

N.B.: I just see that Trac has problems displaying the proto property but I guess you know what I am referring to above...

comment:2 in reply to:  1 Changed 9 years ago by rransom

Replying to gk:

Okay, I just read the comment in jshooks.js today and saw that the commented window.__proto__ = null is causing the issues. Furthermore, I recognized that this problem was already uncovered by Greogory Fleischer some years ago. Nevertheless, I found a solution for fixing it. You have to split the hooks depending on the Firefox version (fun, I know). If the user has a FF3 you may set window.__proto__ = null as usual. If you have a FF4 you do not do this but use the Object.defineProperty with configurable set to false. It's ES5 stuff available in FF4. The code would then be something like:

Object.defineProperty(window.__proto__, "innerWidth", {
             get: function() { return Math.round(origWidth/50.0)*50;}, 
             configurable: false});

in order to fake the innerWidth property. I just tested it and it seems to work. (And yes, an attacker is not able to revert this using things like Object.defineProperty(foo, "bar", { configurable: true}).)

N.B.: I just see that Trac has problems displaying the proto property but I guess you know what I am referring to above...

See [WikiFormatting] (also linked to above the ‘Comment’ box). The only formatting commands you need to include source code fragments properly are the backquotes for inline code and the three-brace structure for blocks of code.

comment:3 Changed 9 years ago by gk

Thanks Robert. I somehow missed that. Anyway, I wanted to add that the hooking method I mentioned above should protect as well against Components.lookupMethod() calls. See: http://www.owasp.org/images/a/a3/Mario_Heiderich_OWASP_Sweden_Locking_the_throneroom.pdf Slide 24. Alas, I was not able to prevent Fleischer's Components.lookupMethod() calls to unmask the screen values. But maybe I was just not smart enough...

comment:4 Changed 9 years ago by mikeperry

Points: 6
Priority: majorcritical

Nice! This is good news. I actually *just* created #2856 to test this new ES5 functionality before reading this ticket. It is great that you made some progress. I will try to get window.screen using the new ES5 features. There's also a ton of other stuff that got added to the DOM, such as some new Range fields that let you get pixel counts for rendered elements, as well as some of the WebGL stuff that needs hooks for fingerprint resistance.

I will leave this ticket open for the actual implementation (which hopefully shouldn't be too much work once we have a good test going, but I'm guessing results may vary by object type).

comment:5 Changed 9 years ago by mikeperry

Keywords: MikePerryIteration20110417 added

comment:6 Changed 9 years ago by mikeperry

Points: 64

comment:7 Changed 9 years ago by mikeperry

Summary: Removing JS hooks in FF4Fix JS Hooks in FF4 using new JS 1.8.5 features

comment:8 Changed 9 years ago by mikeperry

Damnit. You're right. It looks like Firefox totally missed the point of these new ES5 features by allowing Components.lookupMethod to bypass them. I wonder when this changed. It totally seems like Heiderich was planning on relying on the fact that Components.lookupMethod could not bypass these protections. There goes his thrown room...

Also, to make things extra fun, you cannot override Components.lookupMethod itself, as it is set as non-configurable!

I suppose we can just make Components.lookupMethod configurable in our fork of Firefox, and then use this to remove it. We also want to remove Components.interfaces, because all that does is let you fingerprint which Firefox version you have. But guess what: Components.interfaces is also not configurable..

comment:9 in reply to:  8 ; Changed 9 years ago by gk

Replying to mikeperry:

Damnit. You're right. It looks like Firefox totally missed the point of these new ES5 features by allowing Components.lookupMethod to bypass them. I wonder when this changed. It totally seems like Heiderich was planning on relying on the fact that Components.lookupMethod could not bypass these protections. There goes his thrown room...

Indeed, but reading the spec there is actually nothing that prevents things like Components.lookupMethod if the configurable flag is set to false. Thus, I doubt that Mozilla is to blame in this case.

Also, to make things extra fun, you cannot override Components.lookupMethod itself, as it is set as non-configurable!

I almost thought that.

I suppose we can just make Components.lookupMethod configurable in our fork of Firefox, and then use this to remove it. We also want to remove Components.interfaces, because all that does is let you fingerprint which Firefox version you have. But guess what: Components.interfaces is also not configurable..

That sounds interesting do you need a hand here? We certainly have interest in this kind of project as well but not the means to do it alone.

There is another issue I have found. It seems possible under certain circumstances to unhook things if one produces errors during sandbox evaluation. For instance, yesterday I got a lot of errors like "Torbutton Exception in sandbox evaluation. Date hooks not applied: syntax error" while trying to break the hooks somehow. And indeed, at least the innderWidth and innerHeight have been unhooked just because of that (unfortunately I did not test all the other hooked properties and objects). BUT, I have not had luck to produce that behavior again, let alone reliably. Are these errors and its danger a known issue? It seems there is some kind of race condition involved, dunno. Guess I have to dig deeper into the hooking code...

comment:10 in reply to:  9 Changed 9 years ago by mikeperry

Replying to gk:

Replying to mikeperry:

I suppose we can just make Components.lookupMethod configurable in our fork of Firefox, and then use this to remove it. We also want to remove Components.interfaces, because all that does is let you fingerprint which Firefox version you have. But guess what: Components.interfaces is also not configurable..

That sounds interesting do you need a hand here? We certainly have interest in this kind of project as well but not the means to do it alone.

What project are you working on? Forgive me for not recognizing your handle. I created ticket #2873 for our efforts toward a patch.

There is another issue I have found. It seems possible under certain circumstances to unhook things if one produces errors during sandbox evaluation. For instance, yesterday I got a lot of errors like "Torbutton Exception in sandbox evaluation. Date hooks not applied: syntax error" while trying to break the hooks somehow. And indeed, at least the innderWidth and innerHeight have been unhooked just because of that (unfortunately I did not test all the other hooked properties and objects). BUT, I have not had luck to produce that behavior again, let alone reliably. Are these errors and its danger a known issue? It seems there is some kind of race condition involved, dunno. Guess I have to dig deeper into the hooking code...

Back in the Firefox 2/3 days, I went through great effort to try to ensure that the JS hooks applied before anything on the page was able to run. But things may have changed in FF4...

comment:11 Changed 9 years ago by mikeperry

Actual Points: 4
Resolution: fixed
Status: newclosed

Ok, I am going to close this ticket since I've updated origin/master to use this new method. I'm not sure how much we want to do in JShook land.. We're going to have to mull over that for a while.. I've also created a separate ticket (#2934) for experimenting with Date() and Event.timeStamp through JShooks as opposed to patching Firefox.

Note: See TracTickets for help on using tickets.