Hi All,
I'm developing a patch for https://trac.torproject.org/5926, to stop the JavaScript Date object from leaking locale information. I was patching tor-browser.git C++ code, but then I noticed two files in torbutton, jshooks.js and jshooks4.js: https://gitweb.torproject.org/torbutton.git/blob/HEAD:/src/chrome/content/js... https://gitweb.torproject.org/torbutton.git/blob/HEAD:/src/chrome/content/js...
These files seem to be relevant to the patch, and I have some questions about them:
1. In jshooks.js, the series of lines that re-define the window.Date prototype, as controlled by line 72, `if(window.__tb_hook_date == true)`, is never called, as far as I can tell, because __tb_hook_date is never defined. Indeed, if that code were called, then the bug I'm working on would already be mostly fixed. Is there a reason this section of jshooks.js is being retained? Should it be re-activated or is it better for me to continue with a C++ patch?
2. jshooks4.js doesn't seem to be in use at all. Is that correct? Would it make sense to remove this file from the master branch?
3. In general, what's the approach for deciding whether to patch Mozilla C++ code vs torbutton JS code for anti-fingerprinting and similar bugs? I understand there is a desire to move more toward C++, but I'm not sure what the overall plan is.
Thanks for any insight!
Arthur
Hi,
Arthur D. Edelstein:
- In general, what's the approach for deciding whether to patch
Mozilla C++ code vs torbutton JS code for anti-fingerprinting and similar bugs? I understand there is a desire to move more toward C++, but I'm not sure what the overall plan is.
jshooks.js and jshooks4.js are not used anymore mainly due to:
1) there were subtle breakages in the hooks between Firefox versions due to changes in the JS engine code.
2) It turned out that even those sophisticated hooks could get bypassed (see https://bugs.torproject.org/5857). Thus:
"We've given up on JS hooks. They've since been removed." (#5857, comment 6)
So, the current plan is to patch things directly in Fx source code and get that merged upstream in a way usable by us as fast as possible.
Georg
Arthur D. Edelstein:
Hi All,
I'm developing a patch for https://trac.torproject.org/5926, to stop the JavaScript Date object from leaking locale information. I was patching tor-browser.git C++ code, but then I noticed two files in torbutton, jshooks.js and jshooks4.js: https://gitweb.torproject.org/torbutton.git/blob/HEAD:/src/chrome/content/js... https://gitweb.torproject.org/torbutton.git/blob/HEAD:/src/chrome/content/js...
These files seem to be relevant to the patch, and I have some questions about them:
- In jshooks.js, the series of lines that re-define the window.Date
prototype, as controlled by line 72, `if(window.__tb_hook_date == true)`, is never called, as far as I can tell, because __tb_hook_date is never defined. Indeed, if that code were called, then the bug I'm working on would already be mostly fixed. Is there a reason this section of jshooks.js is being retained? Should it be re-activated or is it better for me to continue with a C++ patch?
Right, we abandoned the old Javascript hooking code and disabled it. It proved fragile, difficult to keep up to date, and difficult to protect against various ways of bypassing the hooks. We've opted for direct Firefox patches instead.
- jshooks4.js doesn't seem to be in use at all. Is that correct?
Would it make sense to remove this file from the master branch?
Probably to avoid confusion, yes.
- In general, what's the approach for deciding whether to patch
Mozilla C++ code vs torbutton JS code for anti-fingerprinting and similar bugs? I understand there is a desire to move more toward C++, but I'm not sure what the overall plan is.
Long-term, we want to move all of the privacy features of Torbutton into either C++ patches for Mozilla to merge, or failing that, into a separate addon for dealing specifically with privacy.
In this particular case (locale leaking in Date()), I think we definitely want a C++ patch.
The only thing that might change this is if there were a lot of such locale leaks, and they turned to likely to be both extremely tedious to fix with C++ patches, as well as unlikely to ever be merged by Mozilla.
- jshooks4.js doesn't seem to be in use at all. Is that correct?
Would it make sense to remove this file from the master branch?
Probably to avoid confusion, yes.
I've submitted a patch here for removing the jshooks*.js files. https://trac.torproject.org/projects/tor/ticket/12183
- In general, what's the approach for deciding whether to patch
Mozilla C++ code vs torbutton JS code for anti-fingerprinting and similar bugs? I understand there is a desire to move more toward C++, but I'm not sure what the overall plan is.
Long-term, we want to move all of the privacy features of Torbutton into either C++ patches for Mozilla to merge, or failing that, into a separate addon for dealing specifically with privacy.
In this particular case (locale leaking in Date()), I think we definitely want a C++ patch.
Sounds good.
The only thing that might change this is if there were a lot of such locale leaks, and they turned to likely to be both extremely tedious to fix with C++ patches, as well as unlikely to ever be merged by Mozilla.
This one seems to be doable. Still trying to fix a weird behavior in the associated boolean pref, though, where Preferences::GetBool always returns true, even if the pref is set to false.
Arthur
-
Arthur D. Edelstein -
Georg Koppen -
Mike Perry