Opened 3 years ago

Last modified 13 days ago

#14985 new defect

NoScript Clickjacking warning when clicking on embedded content

Reported by: cypherpunks Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-security
Cc: lunar, gk, gacar, brade, mcs, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

On Tor Browser Bundle 4.5a3, you get a NoScript Clickjacking warning, which you've to allow, and refresh the page, before you're able to fill in a captcha (though probably not only captcha's).

Have seen it with Javascript enabled, disabled, mostly with CloudFlare Captcha's - and Google reCaptcha in general.

Child Tickets

Attachments (1)

clickjacking.png (96.6 KB) - added by cypherpunks 3 years ago.

Download all attachments as: .zip

Change History (19)

Changed 3 years ago by cypherpunks

Attachment: clickjacking.png added

comment:1 Changed 3 years ago by cypherpunks

Priority: normalmajor

comment:2 Changed 3 years ago by gk

Cc: lunar gk added
Keywords: tbb-usability tbb-4.5-regression TorBrowserTeam201505 added; noscript tbb clickjacking captcha removed
Priority: majornormal
Summary: NoScript Clickjacking warning before able to fill in captcha'sNoScript Clickjacking warning when clicking on embedded content

This is not only a CAPTCHA problem, normal banking sites like https://www.labanquepostale.fr (click on the login button right on the top) and embedded pdfs are affected as well.

comment:3 Changed 3 years ago by gk

Ok, so 4.5a3 is indeed the first Tor Browser that breaks things. The Torbutton changes are not the culprit as the clickjacking popup shows up with Torbutton 1.8.1.2, too.

comment:4 Changed 3 years ago by gk

Cc: gacar added

The fix for #13439 is causing this. What fun. Raising prio as there might even more to it.

comment:5 Changed 3 years ago by gk

Priority: normalmajor

comment:6 Changed 3 years ago by mcs

Cc: brade mcs added

comment:7 in reply to:  4 Changed 3 years ago by gacar

I can not seem to understand how. Does the patch in #13439 make NoScript think that the clicked element is hidden, where it's actually not? Would you mind explaining the connection?

Replying to gk:

The fix for #13439 is causing this. What fun. Raising prio as there might even more to it.

comment:8 Changed 3 years ago by arthuredelstein

Cc: arthuredelstein added

comment:9 Changed 3 years ago by mikeperry

gacar - I'm guessing gk just bisected this issue and found the patch at fault. This is easiest to reproduce if you use the security slider in TBB 4.5 to set the security level to High and then to to a cloudflare site (like https://wiki.cyanogenmod.org/) and then click on the captcha with Javascript disabled. The NoScript warning then appears.

For some reason, it only happens for me if Javascript is disabled. Enable it, and everything is fine.

comment:10 Changed 3 years ago by gacar

I could definitely reproduce it on https://www.labanquepostale.fr. I was just looking for pointers to understand the issue.

If it only happens when JS is disabled, that's even more strange. Especially given that NoScript itself accesses canvas to diplay the clickjacking warning.

comment:11 Changed 3 years ago by gk

I am currently debugging it. It is reproducible with a default Tor Browser on https://www.labanquepostale.fr/

comment:12 Changed 3 years ago by gk

So this seems to be a bit tricky. The clearclick dialog shows up due to this.checkObstructed(o, ctx) in ClearClickHandler.js returning false now. However, it is not clear why this happens with the patch in #13439 and not without it. I am still looking for the exact reason. One thing that puzzles me is that I get output like

getfirstPartyURI failed for about:blank: 0x80070057

without the patch in the code path that is crucial for the issue at hand but not with it. Looking at the patch I guess this is because IsCallerChrome() lets us take a shortcut now. I wonder whether ClearClick worked at all in the 4.0.x series as I suspect the fix for #13439 just made a different issue visible. Does anybody have an example of a clickjacking detection by NoScript in a vanilla Firefox we could test in 4.0.x?

comment:13 Changed 3 years ago by gk

It seems my hypothesis is correct: I built both a Tor Browser with

+  if (nsContentUtils::IsCallerChrome())
+    return true;

omitted and one with the bare minimum of patches on top of ESR 31 (basically only the canvas related + the ThirdPartyUtil API ones and some minor .mozconfig tweaks). In both cases there is no clearclick dialog on Lunar's bank's page while the canvas related patches are still working (I still got the popup when visiting github.com).

I might have messed up things with the bare minimum build, though. However, given the time-constraints I propose to just remove the IsCallerChrome() related code snippet as the impact is less grave than all these clearclick false positives (basically #13439 is then an issue again but there should not be any wranings while rendering .pdf files) while bisecting for the real culprit and fixing it for the release after the next one.

comment:14 Changed 3 years ago by mcs

Kathy and I looked at this a little bit. It seems that prior to the #13439 fix, we were preventing NoScript from extracting good image data from the canvas elements that it uses to capture images as part of its clickjacking protection. With the #13439 fix in place, image data is returned (as it should be) but a clickjacking warning is displayed because the wrong portion of the window is being captured. You can see this if you click on the image area of NoScript's clickjacking warning window.

So why is the wrong portion of the image captured? Because of the fix for #5856. Specifically, NoScript's ClearClickHandler.js code relies on getting accurate values for window.mozInnerScreenX and mozInnerScreenY but it receives 0 because the document is content and not chrome.

The #5856 patch is https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-31.6.0esr-4.5-1&id=bd3b1ed32a9c21fdc92fc35f2ec0a41badc378d5

I am not sure what the best fix is, but I suspect that without the #13439 fix, users will not see clickjacking warnings on sites where they should see them. Does anyone know where to find a clickjacking test page?

comment:15 Changed 3 years ago by mikeperry

Keywords: tbb-security added; tbb-usability tbb-4.5-regression removed

I filed #15945 for temporarily disabling the clickjack protection. That ticket is now tagged as the tbb-4.5-regression as well, since the regression is technically that clearclick suddenly started (almost) working in 4.5

comment:16 in reply to:  14 Changed 3 years ago by gk

Replying to mcs:

The #5856 patch is https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-31.6.0esr-4.5-1&id=bd3b1ed32a9c21fdc92fc35f2ec0a41badc378d5

Yes, this was the missing link. Just taking this patch out (+ getting rid of the IsChrome() the screen orientation patch) solves the problem.

comment:17 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201505 removed

comment:18 Changed 13 days ago by teor

Severity: Normal

Set all open tickets without a severity to "Normal"

Note: See TracTickets for help on using tickets.