"Temporarily allow all this page" get inherited when New Identity is chosen.
How to reproduce:
- Open Tor Browser.
- Select "High" Security Setting.
- Go to https://github.com/
- Click on NoScript icon and choose "Temporarilly allow all this page".
- You can note that when the page gets refreshed the canvas prompt is displayed.
- Select New Identity in the Torbutton.
- Go to https://github.com/
- You can notice that the canvas prompt is displayed (meaning JS is enabled, which in turn implies that "Temporarilly allow all this page" gets inherited when New Identity is chosen.)
Activity
I think there's something far more serious under the hood: I was uploading some files to some onion services using the Tor Browser, they (the uploads) timed out, so I refreshed each one and the upload was going again. I then closed the tabs where the upload was happening and I noticed that the upload speed in the Gnome System Monitor was at its max--meaning that the upload was still happening! When I clicked on New Identity, I saw yet again that the upload was still going!! (I had the Gnome System Monitor open while looking at the upload speed) It's only when I closed the Tor Browser that the upload speed returned to its idle value. So it's seems it's not just JS whitelists that get inherited.
Steps to reproduce for comment:5:
- Set High security setting (I was using that one, didn't bother to see if it affects other security settings).
- Go to the Bezos Washington Post Secure Drop: jcw5q6uyjioupxcc.onion (you can verify it here: securedrop.org/directory )
- Click on "Submit Documents".
- Click on "Use New Codename".
- Select some large file (we don't want the upload to finish so we don't waste the journalists' time wasting Bezos' money) like a Tor Browser tar.gz
- Open your Gnome System Monitor and go to "Resources" and watch the network graph. You should see the upload at its peek values.
- Close the tab, you can see that the upload is still going.
- Click on New Identity, again, upload is still going.
- Close the Tor Browser, upload halts.
It works for clearnet websites as well, you can test with https://catbox.moe/ following essentially the same procedure as the last ones above (i.e. 5-9).
Some weird things I learned so far:
-
This got introduced with commit 32f9cf56c89ccd2e24924975dc5515e4198d28c3 which fixes #18913 (moved).
-
This issue (the one in comment:description) is not reproducible during first start with a clean new bundle. However, starting with the second start the STR work for me. Digging deeper what prevents the bug to occur with a clean, new Tor Browser is {{{
// TBB 5.0a3 users had all the necessary data cached in // directoryLinks.json. This meant that resetting the pref above // alone was not sufficient as the tiles features uses the cache // even if the pref indicates that feature should be disabled. // We flip the preference below as this forces a refetching which // effectively results in an empty JSON file due to our spoofed // URLs. let matchOS = m_tb_prefs.getBoolPref("intl.locale.matchOS"); m_tb_prefs.setBoolPref("intl.locale.matchOS", !matchOS); m_tb_prefs.setBoolPref("intl.locale.matchOS", matchOS); }}} which is run intorbutton.json first start.
I am not sure yet how 1) and 2) fit together.
Trac:
Cc: gk to gk, mcs, brade-
#24784 (moved) is a duplicate.
Replying to gk:
mcs, brade do you have some cycles to look at this bug? I probably won't, alas. :(
Kathy and I spent some time debugging this today, but we did not find the root cause. Due to other committments, we may not have much time to work on this until next week, so I am posting what we learned so far here:
- Running with e10s disabled (MOZ_FORCE_DISABLE_E10S=1) avoids the problem.
- I am not convinced it is the cause of this bug because NoScript appears to catch the errors and ignore them, but when running with a debug build when e10s is enabled, several messages like the following are emitted right after the call to
nsSvc.eraseTemp()(called from Torbutton's torbutton_do_new_identity() function):
[Child 23233] ###!!! ASSERTION: ENSURE_MAIN_PROCESS failed. Cannot SetCharPref from content process: temp: 'Error', file /.../dev/tor/tor-browser/modules/libpref/nsPrefBranch.cpp, line 195- I don't know anything about the NoScript internals, but when this bug occurs it seems that
eraseTemp()(inside NoScript's Main.js) is called twice: once directly by Torbutton and once in response to a[purge-session-history](purge-session-history) observer notification. But I don't know if the fact thateraseTemp()` is called twice is causing a problem.
Replying to mcs:
A quick
dump()added to the end of NoScript's Main.js shows it is being loaded twice, but I am not sure if that is by design or not.By Design. Both MainParent and MainChild are built as mix-ins with Main (which is common to both), and this gets loaded twice in the same process if you've got e10s disabled, because of IPC "emulation" (the MessageManager APIs try to show you the same multi-process view of the environment even if e10s is off and everything lives in a single actual process).
-
After some more debugging, Kathy and I have learned that the root of the problem is that some NoScript data structures are not correctly synchronized between the chrome process and the content process when e10s is enabled. Specifically, after "Temporarily allow all this page" is done for a site, one of the
Main.jsinstances does not record the site insidethis.tempSites. Later wheneraseTemp()is called to remove all temporary sites, the site is not removed.I don't know if it is the correct fix, but this bug disappeared after we changed
ns.bootstrapto includesetTempin the list of functions included in theIPC.autoSync()call:try { IPC.autoSync(this, "Main", ["setJSEnabled", "setTemp", "eraseTemp", "allowObject", "resetAllowedObjects", "shutdown"]); } catch (e) {Giorgio, can you take it from here?
-
Replying to mcs:
{{{ try { IPC.autoSync(this, "Main", ["setJSEnabled", "setTemp", "eraseTemp", "allowObject", "resetAllowedObjects", "shutdown"]); } catch (e) { }}}
Giorgio, can you take it from here?
It makes sense, indeed, thank you for your finding. I'm putting it in next 5.x, to be released ASAP.
-
Please check 5.1.6.4rc1 from https://noscript.net/getit#devel, thanks.
-
Replying to ma1:
Please check 5.1.6.4rc1 from https://noscript.net/getit#devel, thanks.
You meant 5.1.8.4rc1, correct? Kathy and I tested it and it fixes this bug. We will leave it up to Georg to decide when to include this version of NoScript in Tor Browser.
-
Replying to ma1:
Replying to mcs:
{{{ try { IPC.autoSync(this, "Main", ["setJSEnabled", "setTemp", "eraseTemp", "allowObject", "resetAllowedObjects", "shutdown"]); } catch (e) { }}}
Giorgio, can you take it from here?
It makes sense, indeed, thank you for your finding. I'm putting it in next 5.x, to be released ASAP.
Giorgio, how's the release going? I don't think we want to put a devel version of NoScript into Tor Browser...
-
Just released 5.1.8.4 as stable. Please also integrate [System+Principal] in your noscript.mandatory preference in order to fix #25000 (moved), thanks.
-
Replying to cypherpunks:
Georg, did you test as well the upload issue?
No. It is not related this this issue. And I am not sure how we should deal with that. See for instance #10426 (moved) which deals with the download case where users are complaining that New Identity does indeed abort the in-progress download.
Trac:
Summary: "Temporarily allow all this page" and uploads get inherited when New Identity is chosen. to "Temporarily allow all this page" get inherited when New Identity is chosen. -
I've opened #25082 (moved) for the upload issue.
mentioned in issue #24479 (moved)
mentioned in issue #24784 (moved)
mentioned in issue #25082 (moved)
mentioned in issue tpo/applications/torbutton#25082 (moved)
mentioned in issue tpo/applications/tor-browser#40962