Opened 9 months ago

Closed 5 months ago

Last modified 5 months ago

#22501 closed defect (fixed)

Requests via javascript: violate FPI

Reported by: cypherpunks Owned by: pospeselr
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-linkability, noscript, TorBrowserTeam201709R
Cc: ma1 Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

With your nightlies http://f4amtbsowhix7rrf.onion/reports/r/tor-browser-2017-06-05/tor-browser_build.html click the link

javascript:togglecontent('test_nightly-linux-x86_64');

near the red cross and get

Torbutton INFO: tor SOCKS: http://f4amtbsowhix7rrf.onion/reports/r/tor-browser-2017-06-05/test_nightly-linux-x86_64 via
                       --unknown--:b3bb40be24aec95a7d8c35608707fc28

Child Tickets

Attachments (3)

22501-pospeselr0.patch (594 bytes) - added by pospeselr 5 months ago.
disables NoScript's fixLinks feature
22501-tor-browser-build-0.patch (2.4 KB) - added by pospeselr 5 months ago.
Put noscript.fixLinks option override in per-platform extension-overrides.js rather than torbutton (since it does not change)
0001-Bug-22501-Requests-via-javascript-violate-FPI.patch (3.3 KB) - added by pospeselr 5 months ago.
Properly formatted patch with a real commit message and everything

Download all attachments as: .zip

Change History (23)

comment:1 Changed 9 months ago by gk

Priority: MediumHigh
Severity: NormalMajor

Nice catch! This happens for me with Tor Browser 6.5.2 already it seems.

comment:2 Changed 9 months ago by gk

Summary: Some requests violate FPIRequests via javascript: violate FPI

I guess this is due to javascript:, adapting the summary accordingly.

comment:3 Changed 9 months ago by cypherpunks

You were asking about new issues so persistently on tor-qa (as if you lack them :) that I couldn't pass by :) How many new issues will be enough for you? What are you going to do with all of them?

comment:4 in reply to:  3 Changed 9 months ago by gk

Replying to cypherpunks:

You were asking about new issues so persistently on tor-qa (as if you lack them :) that I couldn't pass by :) How many new issues will be enough for you? What are you going to do with all of them?

We'll try to get them fixed. Just let them coming.

comment:5 Changed 5 months ago by pospeselr

Owner: changed from tbb-team to pospeselr
Status: newassigned

comment:6 Changed 5 months ago by pospeselr

Extra repro step, need to set security slider to 'Medium'

comment:7 Changed 5 months ago by pospeselr

So the problem here is NoScript with 'noscript.global' preference enabled (hence why only happens when in Medium or Higher security setting).

When an <a> element is clicked and the href attribute starts with 'javascript:' NoScript tries to heuristically extract a URI from the source by looking for a string between " or ' characters that does not contain invalid URI characters ( https://github.com/avian2/noscript/blob/8e12f5ce4f2ddf169da3867ca80323cbbd789948/xpi/chrome/content/noscript/Main.js#L4168 ) and uses that as the href string instead, passing this new href on to an XMLHttpRequest at which point everything happens as normal.

It will interpret the href as relative to the document's URI, unless the href is itself an absolute URL (per https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIIOService#newURI() ).

This has some really cool consequences such that this <a> element will go to github when clicked with NoScript enabled:

<a href="javascript:/* code from 'http://www.github.com' */ window.alert('Hello!');">Hello!</a>

proof: https://pste.eu/p/pWdf.html

comment:8 Changed 5 months ago by gk

Cc: ma1 added
Keywords: noscript added
Status: assignedneeds_information

Thanks for tracking this down. Giorgio: could you have a look at that one? I guess the intended behavior is: *if* we need to issue a request due to clicking on a javascript: link then it should adhere to our first-party isolation. That probably means NoScript itself should not issue that request as this is treated as a browser internal request which gets put onto the catch-all circuit (due to lack of URL bar domain information).

Does that make sense to you, Giorgio?

comment:9 Changed 5 months ago by ma1

It's the "Attempt to fix Javascript links" (noscript.fixLinks in about:config) feature.

Do you need me to modify the HEAD XHR preflight request in order to fit into your navigation restrictions, or to omit it outright? Or would it be better for you to just flip the preference?

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

Status: needs_informationassigned

Replying to ma1:

It's the "Attempt to fix Javascript links" (noscript.fixLinks in about:config) feature.

Do you need me to modify the HEAD XHR preflight request in order to fit into your navigation restrictions, or to omit it outright? Or would it be better for you to just flip the preference?

It seems to me flipping the preference might not be that bad. Richard: can you take a look whether that could be a viable way for solving our issue (without introducing other unintended ones)?

comment:11 Changed 5 months ago by pospeselr

So the noscript.fixLinks will disable the custom onclick handler (which is what does the above described behaviour) but also disables a custom onchange handler (for select and option elements).

However, for Tor Browser that's a good thing, as it has a similar feature whereby it will automatically try to navigate to a selected option if it looks like a URL (the threshold for 'looks like a URL' is even lower though: value contains '/' or '.' and does not contain '@'). This URL will try to be navigated through the same code-path, so would have the same browser internal request gk mentioned.

Updating TorButton to set turn off the noscript.fixLinks option should work, will have a patch up in a bit.

Changed 5 months ago by pospeselr

Attachment: 22501-pospeselr0.patch added

disables NoScript's fixLinks feature

comment:12 Changed 5 months ago by pospeselr

Keywords: TorBrowserTeam201709R added

comment:13 Changed 5 months ago by cypherpunks

Keywords: TorBrowserTeam201709 added; TorBrowserTeam201709R removed
Status: assignedneeds_revision

No, that's for Security Slider options only.

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

Replying to cypherpunks:

No, that's for Security Slider options only.

What else needs to be modified? NoScript settings don't seem to be currently set in torbrowser's 000-tor-browser.js config file.

Changed 5 months ago by pospeselr

Put noscript.fixLinks option override in per-platform extension-overrides.js rather than torbutton (since it does not change)

comment:15 Changed 5 months ago by pospeselr

Verifying works as intended with rbm build.

comment:16 Changed 5 months ago by cypherpunks

Keywords: TorBrowserTeam201709R added; TorBrowserTeam201709 removed
Status: needs_revisionneeds_review

LGTM

comment:17 Changed 5 months ago by pospeselr

Confirmed, noscript.fixLinks now set to 'false' by default in Linux rbm build, --unknown-- domain no longer sent when clicking links on provided page, and URL redirect vulnerability no longer occurs when clicking on javascript:XXX links.

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

Keywords: TorBrowserTeam201709 added; TorBrowserTeam201709R removed
Status: needs_reviewneeds_revision

Replying to pospeselr:

Verifying works as intended with rbm build.

Looks good. The needs_revision is only for making a proper git patch. http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html is not unreasonable as a guideline and we try to follow Mozilla by starting the first line with "Bug XXXXX" ("XXXXX" being the bug number the commit relates to).

Changed 5 months ago by pospeselr

Properly formatted patch with a real commit message and everything

comment:19 Changed 5 months ago by gk

Resolution: fixed
Status: needs_revisionclosed

Thanks. Applied to master (commit ee2f06091d76272c7b629265d8c0a67c3d3b07e1).

comment:20 Changed 5 months ago by gk

Keywords: TorBrowserTeam201709R added; TorBrowserTeam201709 removed
Note: See TracTickets for help on using tickets.