NoScript Clickjacking warning when clicking on embedded content

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.

added component::applications/tor browser noscript owner::tbb-team priority::high severity::normal status::new tbb-security type::defect labels

Trac:

Trac:
Priority: normal to major

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.

Trac:
Summary: NoScript Clickjacking warning before able to fill in captcha's to NoScript Clickjacking warning when clicking on embedded content
Keywords: noscript, clickjacking, captcha, tbb deleted, tbb-usability, tbb-4.5-regression, TorBrowserTeam201505 added
Cc: N/A to lunar, gk
Priority: major to normal

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.

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

Trac:
Cc: lunar, gk to lunar, gk, gacar

Trac:
Priority: normal to major

Trac:
Cc: lunar, gk, gacar to lunar, gk, gacar, brade, mcs

I can not seem to understand how. Does the patch in #13439 (moved) 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 (moved) is causing this. What fun. Raising prio as there might even more to it.

Trac:
Cc: lunar, gk, gacar, brade, mcs to lunar, gk, gacar, brade, mcs, arthuredelstein

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.

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.

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

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 (moved) 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 (moved) 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?

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 (moved) 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.

Kathy and I looked at this a little bit. It seems that prior to the #13439 (moved) 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 (moved) 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 (closed). 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 (closed) 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 (moved) fix, users will not see clickjacking warnings on sites where they should see them. Does anyone know where to find a clickjacking test page?

I filed #15945 (moved) 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

Trac:
Keywords: tbb-usability, tbb-4.5-regression deleted, tbb-security added

Replying to mcs:

The #5856 (closed) 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.

Trac:
Keywords: TorBrowserTeam201505 deleted, N/A added

Set all open tickets without a severity to "Normal"

Trac:
Severity: N/A to Normal

Trac:
Keywords: N/A deleted, noscript added
Sponsor: N/A to N/A
Reviewer: N/A to N/A

mentioned in issue #15945 (moved)

moved to tpo/applications/tor-browser#14985 (closed)

mentioned in issue tpo/applications/tor-browser#15945 (closed)