Opened 5 months ago

Closed 3 months ago

#22362 closed defect (fixed)

Tor Browser freezes when loading https://www.facebook.com/tr/ on a website

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: noscript, GeorgKoppen201707, TorBrowserTeam201707
Cc: ma1, fdsfgs@…, isis, jiminycricket, TorontoBoy Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by gk)

Tor Browser in its default mode (i.e. with JavaScript allowed) freezes when loading http://zeit.de. This happens every time when requesting https://www.facebook.com/tr/.

My console output stops always with

[05-23 16:59:52] Torbutton INFO: tor SOCKS: https://www.facebook.com/tr/ via
                       zeit.de:a137cc9014c3589a23fe6c5d354a4d7d

After a bit of debugging it turns out that vanilla Firefox 52 versions are affected as well but only if NoScript is installed. This happens with NoScript 5.0.4 at least.

Child Tickets

Change History (34)

comment:1 Changed 5 months ago by gk

Giorgio: let me know if you have issues with reproducing this. I have saved a local copy of the website and could send it to you as I can see the issue with that copy as well.

comment:2 Changed 5 months ago by gk

Description: modified (diff)

comment:3 Changed 5 months ago by gk

Description: modified (diff)

comment:4 Changed 5 months ago by tokotoko

Cc: fdsfgs@… added

comment:5 Changed 5 months ago by gk

Cc: isis added
Priority: MediumHigh
Severity: NormalMajor
Summary: Tor Browser freezes when loading http://zeit.deTor Browser freezes when loading https://www.facebook.com/tr/ on a website

Happens with https://www.theatlantic.com/business/archive/2016/07/racist-history-portland/492035/?utm_source=twb as well. The only workaround I have so far is disabling NoScript. :( #22438 is a duplicate.

comment:6 in reply to:  5 Changed 5 months ago by teor

Replying to gk:

Happens with https://www.theatlantic.com/business/archive/2016/07/racist-history-portland/492035/?utm_source=twb as well. The only workaround I have so far is disabling NoScript. :( #22438 is a duplicate.

To clarify, one workaround is to disable all JavaScript *using* NoScript, or to use Tor's high security mode.

comment:7 Changed 5 months ago by gk

I just updated to NoScript 5.0.5 which does not solve the problem.

comment:8 Changed 5 months ago by ma1

I'm checking it right now.
In the meanwhile, would deploying an ABE rule like this one:

Site https://www.facebook.com/tr/
Allow from SELF
Deny

work?

comment:9 Changed 5 months ago by gk

Thanks. Let me see. FWIW: Enabling debug output gives me

[NoScript C] Document OK: @https://www.facebook.com/tr/ --- PGFM: null
[NoScript C] Document OK: @https://www.facebook.com/tr/ --- PGFM: null
[NoScript C] Sync on pageshow javascript:false

"Sync on pageshow javascript:false" is only visible for https://www.facebook.com/tr/. Might be related to this bug?

comment:10 in reply to:  8 ; Changed 5 months ago by gk

Replying to ma1:

I'm checking it right now.
In the meanwhile, would deploying an ABE rule like this one:

Site https://www.facebook.com/tr/
Allow from SELF
Deny

work?

ABE complains in that case. It seems it does not know "Allow". But replacing that with "Accept" makes it happy. Let me know if that's not the right thing. However, after restarting and with that rule in place I still get the hangs.

comment:11 in reply to:  10 Changed 5 months ago by ma1

Replying to gk:

Replying But replacing that with "Accept" makes it happy.
However, after restarting and with that rule in place I still get the hangs.

Yes, it's "Accept", my bad.

It doesn't seem caused by the load (it's just a 1 pixel GIF), but maybe just correlated and, I suspect, caused by a script checking for something that's not there in a loop.

Going on with the investigation and looking for work-arounds...

comment:12 Changed 5 months ago by gk

Cc: jiminycricket added

Marked #22435 as a duplicate.

comment:13 Changed 5 months ago by ma1

It was https://www.facebook.com/tr/, after all.

It gets loaded several times with GET requests, but at a certain point a POST request containing two big JSON parameter is sent, and this apparently hangs the XSS filter (which is triggered BEFORE ABE, hence the futility of my previous proposed work around).

While I try to understand what actually happens in the guts of the filter, a new work around which WFM is adding the following line to NoScript Options>Advanced>XSS exceptions box (about:config -> noscript.filterXExceptions):

^https://www\.facebook\.com/tr/

comment:14 in reply to:  13 Changed 5 months ago by gk

Replying to ma1:

It was https://www.facebook.com/tr/, after all.

It gets loaded several times with GET requests, but at a certain point a POST request containing two big JSON parameter is sent, and this apparently hangs the XSS filter (which is triggered BEFORE ABE, hence the futility of my previous proposed work around).

Woah. Thanks for looking into it. The workaround does the job for me as well.

comment:15 in reply to:  13 Changed 5 months ago by gk

Status: newneeds_information

Replying to ma1:

While I try to understand what actually happens in the guts of the filter, a new work around which WFM is adding the following line to NoScript Options>Advanced>XSS exceptions box (about:config -> noscript.filterXExceptions):

^https://www\.facebook\.com/tr/

ma1: Do you have an ETA for a new NoScript version with a fix for this problem? I am a bit reluctant to add that exception to the list for Tor Browser. But we are currently preparing Tor Browser 7.0, so having this issue solved some time next week or early in the week thereafter would rock.

comment:16 Changed 5 months ago by gk

Keywords: TorBrowserTeam201706 added

comment:17 Changed 5 months ago by gk

After thinking a while about how to mitigate that problem in Tor Browser while we are waiting for a new NoScript release it seems to me we don't want to just whitelist that particular tracker. The risk is that a week after release a different one hits the same bug and we start playing whack-a-mole. I think we should disable the XSS protection for now until it can't be used as a DoS tool anymore.

comment:18 Changed 5 months ago by gk

Keywords: TorBrowserTeam201706R GeorgKoppen201706 added; TorBrowserTeam201706 removed
Status: needs_informationneeds_review

comment:19 Changed 5 months ago by jiminycricket

Is this the same issue persisting in Tor Browser 7.0? The Guardian also crashes on this page:

https://www.theguardian.com/science/2017/jun/07/oldest-homo-sapiens-bones-ever-found-shake-foundations-of-the-human-story

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

Replying to jiminycricket:

Is this the same issue persisting in Tor Browser 7.0? The Guardian also crashes on this page:

https://www.theguardian.com/science/2017/jun/07/oldest-homo-sapiens-bones-ever-found-shake-foundations-of-the-human-story

Yes. We'll ship a workaround in 7.0.1 which is coming out next week.

comment:21 in reply to:  18 Changed 5 months ago by mcs

Replying to gk:

bug_22326 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_22362&id=e8178d1373d6ce2599ef58eca4d52d1b6817263f) has a patch for the workaround in comment:17 up for review.

r=mcs
This looks good to me, and it solved the problem (at least on OSX for the page mentioned in comment:19).

Last edited 5 months ago by mcs (previous) (diff)

comment:22 Changed 5 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks. Let's ship that workaround for now then. Merged to master (commit e8178d1373d6ce2599ef58eca4d52d1b6817263f) and cherry-picked to maint-7.0 (commit f0584bcbec0f5c15ad510368df5088ce64ec48b6).

comment:23 Changed 4 months ago by gk

Cc: TorontoBoy added

Marked #22561 as duplicate.

comment:24 Changed 4 months ago by gk

#22406 was a duplicate.

comment:25 Changed 4 months ago by ma1

Resolution: fixed
Status: closedreopened

Of course disabling the XSS filter is an undesirable work-around.

Turns out that this bug was due to the JSON-stripping optimization being completely turned off by a regression.

Should be fixed in latest development build 5.0.6rc5, please check thanks.

comment:26 Changed 4 months ago by gk

Keywords: TorBrowserTeam201707 added

Moving Tickets to July 2017.

comment:27 Changed 4 months ago by gk

Keywords: GeorgKoppen201707 added; GeorgKoppen201706 removed

Moving my tickets to July 2017.

comment:28 Changed 4 months ago by gk

Keywords: TorBrowserTeam201707R added; TorBrowserTeam201706R removed

Moving the review tickets to July 2017 as well.

comment:29 Changed 4 months ago by gk

Keywords: TorBrowserTeam201707 removed

comment:30 in reply to:  25 Changed 3 months ago by gk

Keywords: TorBrowserTeam201707 added; TorBrowserTeam201707R removed
Status: reopenedneeds_revision

Replying to ma1:

Of course disabling the XSS filter is an undesirable work-around.

Turns out that this bug was due to the JSON-stripping optimization being completely turned off by a regression.

Should be fixed in latest development build 5.0.6rc5, please check thanks.

Testing with 5.0.6 seems to indicate that there is still something not working as expected:

[07-10 13:12:00] Torbutton INFO: tor SOCKS: https://www.facebook.com/tr/ via
                       zeit.de:73bb9561126604354e4dfe26f54a59dd
SyntaxError: invalid range in character class
InjectionChecker.reduceJSON@chrome://noscript/content/InjectionChecker.js:167:15
InjectionChecker_checkJSBreak@chrome://noscript/content/InjectionChecker.js:475:30
InjectionChecker.checkJS@chrome://noscript/content/InjectionChecker.js:748:70
InjectionChecker._checkRecursive@chrome://noscript/content/InjectionChecker.js:997:30
InjectionChecker.checkRecursive@chrome://noscript/content/InjectionChecker.js:992:12
InjectionChecker.checkRecursive@chrome://noscript/content/InjectionChecker.js:967:33
InjectionChecker.checkPostStream/<@chrome://noscript/content/InjectionChecker.js:1165:11
PostChecker.prototype.check@chrome://noscript/content/InjectionChecker.js:1259:17
InjectionChecker.checkPostStream@chrome://noscript/content/InjectionChecker.js:1162:13
InjectionChecker.checkPost@chrome://noscript/content/InjectionChecker.js:1156:12
RequestWatchdog.prototype.filterXSS@chrome://noscript/content/RequestWatchdog.js:789:46
RequestWatchdog.prototype.onHttpStart/<@chrome://noscript/content/RequestWatchdog.js:148:18
DOSChecker.prototype.run@chrome://noscript/content/RequestWatchdog.js:1104:22
RequestWatchdog.prototype.onHttpStart@chrome://noscript/content/RequestWatchdog.js:146:9
MainParent["http-on-modify-request"].observe@chrome://noscript/content/MainParent.js:82:24

TypeError: unsafeRequest.window is null

Just opening zeit.de and waiting should give you that syntax error.

comment:31 Changed 3 months ago by ma1

Please check 5.0.7rc1, thanks.

comment:32 Changed 3 months ago by gk

Looks better. Please let us know when 5.0.7 will be out so that we can revert our workaround and close this ticket properly.

comment:33 Changed 3 months ago by cypherpunks

5.0.8.1 is out and looks better.

comment:34 Changed 3 months ago by gk

Resolution: fixed
Status: needs_revisionclosed

I reverted our workaround with commit 47035611306843657a5847d31fd4b5600742127a on master and commit b9e83cebe097c3c3ef7454ce26310ae672de2490 on maint-7.0.

Note: See TracTickets for help on using tickets.