Opened 6 weeks ago

Closed 5 weeks ago

#29733 closed defect (fixed)

Disable NoSript XSS protection for now due to bug 1532530

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: noscript, TorBrowserTeam201903
Cc: ma1 Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

It seems there is a Firefox bug in relation to NoScript's XSS feature that makes large uploads stall which is bad for SecureDrop, Onionshare and others. We'll therefore disable the XSS feature for now until this gets fixed.

Child Tickets

Change History (20)

comment:1 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201903R added; TorBrowserTeam201903 removed
Status: newneeds_review

comment:2 Changed 6 weeks ago by ma1

For reference, the upstream Mozilla bug is https://bugzilla.mozilla.org/show_bug.cgi?id=1532530

This seems exceedingly drastic as a work-around.
What if I provide an option to just disable XSS injection checks on POST parameters (which would prevent the requestBody listener from being registered), and possibly another option to ask user confirmation for POST requests from JavaScript-disabled sites to TRUSTED ones, in order to mitigate the loss of protection?

comment:3 in reply to:  2 Changed 6 weeks ago by gk

Replying to ma1:

For reference, the upstream Mozilla bug is https://bugzilla.mozilla.org/show_bug.cgi?id=1532530

This seems exceedingly drastic as a work-around.

Well, OnionShare and SecureDrop are important tools, especially for people in dangerous situations. The risk here is that they mess up by not understanding the workarounds done by others or using a different unsafe tool altogether. We should avoid those failure cases.

What if I provide an option to just disable XSS injection checks on POST parameters (which would prevent the requestBody listener from being registered), and possibly another option to ask user confirmation for POST requests from JavaScript-disabled sites to TRUSTED ones, in order to mitigate the loss of protection?

I think that would work for me (even though I admit that I was looking forward to "solve" the XSS related freezes with the patch, too :) (see: #29647 for details)) as long as it works for the SecureDrop/OnionShare users.

comment:4 Changed 6 weeks ago by gk

Giorgio: please let me know what your plans/timeline for the workaround is if you know that already. I suspect we want to start building the new Tor Browser tomorrow and I think the issue in this bug warrants some action. Thus, we'd like to have some help for the OnionShare/SecureDrop users in place, say, by next Tuesday/Wednesday (be it either by the envisioned patch in this bug or a different workaround). Thanks!

comment:5 in reply to:  4 ; Changed 6 weeks ago by ma1

Replying to gk:

Giorgio: please let me know what your plans/timeline for the workaround is if you know that already.

I can try to provide you with a build (most likely a RC with just this work-around) by tomorrow.

comment:6 in reply to:  5 ; Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201903 added; TorBrowserTeam201903R removed
Status: needs_reviewnew

Replying to ma1:

Replying to gk:

Giorgio: please let me know what your plans/timeline for the workaround is if you know that already.

I can try to provide you with a build (most likely a RC with just this work-around) by tomorrow.

Thanks! We'll start building Tor Browser release candidates without the patch up for review then, assuming the workaround will happen in NoScript directly.

comment:7 in reply to:  6 Changed 6 weeks ago by ma1

Replying to gk:

Thanks! We'll start building Tor Browser release candidates without the patch up for review then, assuming the workaround will happen in NoScript directly.

Actually I still need some way to tell we're in the Tor Browser and/or we need this work-around, because I'm not making it the default for all the NoScript users.

May I suggest changing this patch into just

-   "tabId": -1
+    "torBrowser": Services.appinfo.platformVersion,
+    "tabId": -1

This would help a lot in similar situations.
Not surprisingly, because it would be a way to fix issue #29021 ;)

Thank you!

comment:8 Changed 6 weeks ago by gk

Oh, good point. Do you need the platformVersion here? Otherwise I'd just like to avoid the overhead of querying that one and would just go with your idea mentioned in #29021, doing something like:

     },
+   "isTorBrowser": true,
    "tabId": -1
   });

comment:9 in reply to:  8 Changed 6 weeks ago by ma1

Replying to gk:

Do you need the platformVersion here? Otherwise I'd just like to avoid the overhead of querying that one and would just go with your idea mentioned in #29021, doing something like:

     },
+   "isTorBrowser": true,
    "tabId": -1
   });

That's fine, thanks. I can use browser.runtime.getBrowserInfo() if I also need version information.

comment:10 Changed 6 weeks ago by gk

Okay, #29021 is done. I'll tag a -build2 shortly to include that into our upcoming stable, too.

comment:11 Changed 6 weeks ago by eloquence

What if I provide an option to just disable XSS injection checks on POST parameters (which would prevent the requestBody listener from being registered), and possibly another option to ask user confirmation for POST requests from JavaScript-disabled sites to TRUSTED ones, in order to mitigate the loss of protection?

What will the default behavior in Tor be if, say, the user is attempting to upload to SecureDrop with JavaScript disabled? Would they get a scary confirmation dialog? It would be really good to avoid scary confirmation messages that make the user think that there is a security issue, when there really is not.

(I realize this is now a NoScript issue again, feel free to point me to a corresponding issue if that's a better place to discuss. :)

comment:12 in reply to:  11 Changed 6 weeks ago by ma1

Here's the RC containing the work-around:

https://github.com/hackademix/noscript/releases/tag/10.2.2rc3

Replying to eloquence:

What will the default behavior in Tor be if, say, the user is attempting to upload to SecureDrop with JavaScript disabled?

Nothing visible should happen.

Would they get a scary confirmation dialog?

Not in your case, unless I'm missing something. Please let me know if I'm wrong.
NoScript should show a (not so scary) confirmation dialog only for cross-site requests with the destination enabled to run scripts (since it replaces a more specific anti-cross-site-scripting protection).

(I realize this is now a NoScript issue again, feel free to point me to a corresponding issue if that's a better place to discuss. :)

Here's the best place until Mozilla fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1532530 (which I hope they will: today they assigned the bug to the proper developer).

comment:13 in reply to:  11 Changed 6 weeks ago by gk

Status: newneeds_information

Replying to eloquence:

What if I provide an option to just disable XSS injection checks on POST parameters (which would prevent the requestBody listener from being registered), and possibly another option to ask user confirmation for POST requests from JavaScript-disabled sites to TRUSTED ones, in order to mitigate the loss of protection?

What will the default behavior in Tor be if, say, the user is attempting to upload to SecureDrop with JavaScript disabled? Would they get a scary confirmation dialog? It would be really good to avoid scary confirmation messages that make the user think that there is a security issue, when there really is not.

(I realize this is now a NoScript issue again, feel free to point me to a corresponding issue if that's a better place to discuss. :)

Could you test whether the Tor Browser release candidate (https://people.torproject.org/~boklm/builds/8.0.7-build3/ has bundles) + the NoScript release candidate solve the issue for you?

comment:14 Changed 6 weeks ago by eloquence

Here's the procedure I followed:

1) Per last comment on https://trac.torproject.org/projects/tor/ticket/29733 , downloaded 8.0.7b3 from https://people.torproject.org/~boklm/builds/8.0.7-build3/tor-browser-linux64-8.0.7_en-US.tar.xz and ran it

2) Removed shipped version of NoScript, activated debug mode, downloaded Source Code ZIP from https://github.com/hackademix/noscript/releases/tag/10.2.2rc3 , and loaded its manifest.json in debug mode

3) Changed NoScript settings to these ones: "Sanitize cross-site suspicious requests": CHECKED, "Scan uploads for potential cross-site attacks": NOT CHECKED, "Ask confirmation for cross-site POST requests which could not be scanned": CHECKED

4) Uploaded a 271M file through source interface of my local SecureDrop hardware instance.

So far so good -- two test uploads succeeded, will do some more testing tomorrow. I'll flag this to the OnionShare folks in case they have time to do additional testing, as well.

Thanks for all the help getting this issue resolved. Fingers crossed; will post another update after more tests.

comment:15 Changed 6 weeks ago by mig5

Hi, mig5 from the OnionShare project here.

Following eloquence's steps, I can confirm that OnionShare uploads are succeeding for me too, using that TB 8.0.7-build3 build, and 10.2.2rc3 of NoScript.

I tested uploading a 62, 125 and 377 MB file, both in 'local' (no Tor) and Tor mode, all under Tor Browser's 'Safer' security setting, without experiencing any issues. Looking good, thanks!

comment:16 Changed 6 weeks ago by eloquence

Did a bit more 127.0.0.1 testing in this version of Tor as well (using Micah's upload server script: https://github.com/micahflee/noscript-upload-bug) and can further confirm that 1) As expected, I can't reproduce the issue with the "Scan uploads for potential cross-site attacks" checkbox unchecked; 2) As expected, I can reproduce it quickly with that checkbox checked.

As long as the preferences are indeed set correctly in the shipped version, I think we're good to go as far as this bug is concerned. :)

comment:17 Changed 5 weeks ago by gk

ma1: I tested 8.0.7 with 10.2.2 and realized that I am now seeing for any search request typed in the URL bar a scary XSS warning popup. That's very unfortunate as there is definitely no XSS involved if I type my search queries into the URL bar. Could you please fix that?

comment:18 in reply to:  17 ; Changed 5 weeks ago by ma1

Replying to gk:

ma1: I tested 8.0.7 with 10.2.2 and realized that I am now seeing for any search request typed in the URL bar a scary XSS warning popup. That's very unfortunate as there is definitely no XSS involved if I type my search queries into the URL bar. Could you please fix that?

Fixed in NoScript 10.2.3rc2.

comment:19 in reply to:  18 ; Changed 5 weeks ago by ma1

Replying to ma1:

Replying to gk:

ma1: I tested 8.0.7 with 10.2.2 and realized that I am now seeing for any search request typed in the URL bar a scary XSS warning popup. That's very unfortunate as there is definitely no XSS involved if I type my search queries into the URL bar. Could you please fix that?

Fixed in NoScript 10.2.3rc2.

Now also in 10.2.3, in case you've got some "ship stable releases only" policy.

Last edited 5 weeks ago by ma1 (previous) (diff)

comment:20 in reply to:  19 Changed 5 weeks ago by gk

Resolution: fixed
Status: needs_informationclosed

Replying to ma1:

Replying to ma1:

Replying to gk:

ma1: I tested 8.0.7 with 10.2.2 and realized that I am now seeing for any search request typed in the URL bar a scary XSS warning popup. That's very unfortunate as there is definitely no XSS involved if I type my search queries into the URL bar. Could you please fix that?

Fixed in NoScript 10.2.3rc2.

Now also in 10.2.3, in case you've got some "ship stable releases only" policy.

Yes, thanks for that. I bumped the NoScript version to the latest stable one in commits fe57b321785474679b6adadcf769eb08dde28f76 and 37aa44ee2954bd99e9a53cf00cb4b474b86a07fb on master and in commit 378de243109024a80e841bfa47efcca5d7a5c18f on maint-8.0 in our tor-browser-build repo. It's a bit unfortunate that there are now many more false positive popups disrupting the user experience. So we'll need to monitor this and re-think enabling XSS protections if we come to the conclusion that enabling them outweigh the usability penalties. (#29647 and above all #26847 come to mind here)

Anyway, thanks Giorgio for the quick help!

Note: See TracTickets for help on using tickets.