1380616 - "Firefox Screenshots" functionality can be tricked into injecting trusted UI into an untrusted frame

Status: RESOLVED FIXED

(Firefox :: Screenshots, defect)

Description

[Wladimir Palant]

Attachment: Proof of concept

The Firefox Screenshots feature injects its user interface into the pages. The general approach is correct: the feature's user interface is separated from the page content via a moz-extension://.../blank.html frame that the page cannot access. However, the extension will not check the frame's URL, so the page can load about:blank into the frame and the same trusted user interface will be injected. This allows the page to automate the user interface, only limited by the few event.isTrusted checks in the code.

Steps to reproduce:

1. Make sure to use "Firefox Screenshots" feature at least once, so that the tour no longer appears. The proof of concept might still work with the tour appearing but I didn't test it in this scenario.
2. Open the attached proof of concept page.
3. Right-click the page, choose "Take a Screenshot" from the context menu.
4. Wait a second, then click anywhere on the page.

Expected results:

The screenshot UI doesn't appear (no way to prevent the page from hiding the frames) but the click doesn't have any effect either.

Actual results:

In Firefox 56.0a1 nightly (2017-07-12), clicking the page saves the screenshot to "My Shots", opens a new tab for it and displays a message about the contents of my clipboard being overwritten. This is because the page replace frame URLs by about:blank which allowed it to manipulate the contents - trigger the necessary event handlers to select a part of the page, then change the "Save" button occupy the entire page while staying transparent (performing the same manipulation with the "Download" button would have been possible as well). It still needs the user to click it however (not a high hurdle) because the button's click handler will check event.isTrusted and reject generated events.

Recommendations:

In extension's webextension/selector/ui.js, the onload event handlers defined by iframeSelection.display() and iframePreSelection.display() should check that this.element.contentDocument.URL is still browser.extension.getURL("blank.html") - this handler shouldn't initialize documents with unexpected URLs. Also, it should be a good idea to make sure that this handler runs only once per frame (remove after it is triggered for the first time). Finally, defense in the depth should be consistent - no event handlers should accept untrusted events.

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

What's the worst case impact here for there user - am I correct in thinking that it's limited to adding unwanted screenshot ? Reccomendations seem to make sense regardless but I'd like to better understand the impact - worst case would be the ability to streak the users screenshot Uris but I don't think that is possible through this bug. (I'm travelling though, can someone t st this further ?)

Comment 2

[Wladimir Palant]

Actually, with the URL copying taking place within the affected frame (textarea element being added to the document), the value copied to clipboard can be intercepted by the page. The page can then download the data on the server side. While this still doesn't seem to allow identifying the user or their other screenshots, it gives the website access to its own screenshot. For reference, canvas.drawWindow() is forbidden for content for a reason, with history leaks being one possible implication (website can distinguish visited and unvisited links in the screenshot by their color).

Other than that, adding/downloading unwanted screenshots seems to be mostly it. The page can also mess with data-l10n-id attributes for onboarding which allows reading out localization data (including predefined messages such as @@ui_locale), but that's probably information that the page can get by other means already. "My Shots" button opens a new tab so that data is out of reach.

Comment 3

[Wladimir Palant]

(In reply to Wladimir Palant from comment #2)
> Actually, with the URL copying taking place within the affected frame
> (textarea element being added to the document), the value copied to
> clipboard can be intercepted by the page. The page can then download the
> data on the server side. While this still doesn't seem to allow identifying
> the user or their other screenshots, it gives the website access to its own
> screenshot. For reference, canvas.drawWindow() is forbidden for content for
> a reason, with history leaks being one possible implication (website can
> distinguish visited and unvisited links in the screenshot by their color).

Actually, that is even more problematic than I thought. It seems that this feature will happily create screenshots of third-party frames. So there you have your data leak - the page can load http://192.168.0.1/ in a frame, make a screenshot of it and get a look at the UI of my home router (similarly for some website on a company intranet). Luckily, major sites like Google or Facebook don't allow framing, otherwise one could deanonymize users this way.

Comment 4

[Julien Vehent]

Thank you Wladimir for the analysis. I'm raising it to sec-high for now given your latest comment.
Greg: could you please verify this issue? I think Freddy can help, but we're nearing EOD for him.

Comment 5

[Frederik Braun [:freddy]]

Yes, taking screenshots of other origins is sec-high. As discussed on IRC, Greg will continue here.
I can drop in later tonight, or follow up tomorrow, just let me know.

Comment 6

[Greg G]

On the POC page the screenshots overlay UI doesn't appear after a right-click and selecting "Take a Screenshot" from the context menu.

Waiting a second then clicking on the POC page opens a new tab with a screenshot of the top left of the page: https://screenshots.firefox.com/sucdjN4GalecPGEi/bug1380616.bmoattachments.org

(for nightly 2017-07-12)

Comment 7

[Wladimir Palant]

Attachment: Proof of concept with clipboard hijacking

I attached an extended proof of concept. The steps to reproduce are unchanged compared to comment 0, only difference is that now the screenshot will contain the contents of a frame pointing to example.com (displayed below the fold) and after creating the screenshot the page will display its URL. So in theory this page could screenshot any third-party frame and access the contents of this screenshot.

Additional recommendation: copying to clipboard should happen inside a frame, not inside the page itself! A page can currently intercept the URL of any screenshot created by the user even without messing with the frames. Of course, ideally copying to clipboard in the background page were fixed...

Comment 8

[Greg G]

I can confirm I'm seeing a screenshot of example.com https://screenshots.firefox.com/VTvMTKP1B9wJy8ap/localhost when I serve the extended POC locally and run through the steps in https://bugzilla.mozilla.org/show_bug.cgi?id=1380616#c0.

+1 for the recommendations in comments 0 and 7

Comment 9

[Julien Vehent]

Wladimir: just to be 100% clear, are you able to download the resulting screenshot?

Comment 10

[Daniel Veditz [:dveditz]]

(In reply to Frederik Braun [:freddyb] from comment #5)
> Yes, taking screenshots of other origins is sec-high.

Maybe: arbitrary same-origin violations are definitely sec-high, but in this case you have to first convince the user to screenshot something which is not common. Feels more sec-moderate to me.

(In reply to Julien Vehent [:ulfr] from comment #9)
> Wladimir: just to be 100% clear, are you able to download the resulting screenshot?

Aren't screenshots public by nature so you can share the links?

Comment 11

[Julien Vehent]

> you have to first convince the user

That's probably easy to achieve with a bit of "try this new Firefox feature" phishing.

> Aren't screenshots public by nature so you can share the links?

Assuming you know the URL the screenshot is at, yes. So if the attack can retrieve the URL, it can access the shot.

Comment 12

[Greg G]

The extended POC updates to include the screenshot URL e.g. "Screenshot created under https://screenshots.firefox.com/tjM6E08aYe4m48Fz/localhost"

This is the basis of the "copying to clipboard should happen inside a frame" or don't copy in the background recommendation.

Comment 13

[Ian Bicking (:ianbicking)]

Here's the fixes we need, as I understand the comments here:

1. Move the clipboard textarea into an appropriate iframe (probably a new iframe separate from the others)
2. Put in checks for all iframe creation, that when iframe onload happens we have an iframe with the expected URL
3. Remove the onload handler after creation
4. Add some more assertIsTrusted calls, specifically in uicontrol.js

We'll work off the v10.4.0-export branch (which represents what's currently in Firefox), as we've changed some of the iframe code in master, and I want to start with patches that can easily be exported into Firefox. We'll have to port them to master later.

Comment 14

[Wladimir Palant]

(In reply to Julien Vehent [:ulfr] from comment #9)
> Wladimir: just to be 100% clear, are you able to download the resulting
> screenshot?

Yes. The screenshot itself is public, the only missing part is its URL - and the extended PoC gets that from the code copying to clipboard. It actually displays the screenshot URL on the page.

(In reply to Daniel Veditz [:dveditz] from comment #10)
> but in this
> case you have to first convince the user to screenshot something which is
> not common.

Sure, social engineering but convincing the user to click the screenshot button is easy - it's not like anybody expects that functionality to cause trouble. And instead of merely hiding the UI, a web page could show something fancy. If anything, the issue with the attack here is it being very noisy. It will open a new tab and display a notification, there doesn't seem to be a way for the website to prevent it - not impossible to convince the user that these are legit, but this definitely makes it harder.

Comment 15

[Wladimir Palant]

Attachment: Proof of concept for clipboard hijacking only

As I mentioned in comment 7, copying the screenshot URL into the clipboard currently happens in the context of the page rather than some secure frame. This allows web pages to hijack the process, read out screenshot URL and even replace it; this is regardless of the weakness I reported initially. I've attached a third proof of concept showing this vulnerability only. Steps to reproduce:

1. Open "Proof of concept for clipboard hijacking only" page.
2. Right-click the page, choose "Take a Screenshot" from the context menu.
3. Save a screenshot.
4. Go back to the page.

Expected results:

The page cannot know the URL of the screenshot you just saved. Also, the URL of the screenshot is in your clipboard like the notification claimed.

Actual results:

The page shows the URL of your newly created screenshot. Your clipboard contains the URL http://malicious.example.info/.

Comment 16

[Daniel Veditz [:dveditz]]

(In reply to Wladimir Palant from comment #14)
> Sure, social engineering but convincing the user to click the screenshot
> button is easy - it's not like anybody expects that functionality to cause
> trouble. [...] If anything, the issue with the attack here is it being
> very noisy. It will open a new tab and display a notification

Agreed. I wasn't saying this attack wouldn't work on some people, or even a lot of them. Was trying to assess that those factors limit the usefulness of this as a mass attack vector (the alarm would be raised quickly). Given the power to capture cross-origin frames its still an attractive vulnerability to use in more targeted attacks.

Comment 17

[Ian Bicking (:ianbicking)]

Attachment: Proposed XPI with security changes

Attaching an XPI with our proposed changes. We should first confirm this fixes the security problems, then we'll ask QA to confirm it doesn't otherwise cause regressions.

Comment 18

[Ian Bicking (:ianbicking)]

Attachment: security-fixes.patch

This patch is only informational (not meant to be applied), and gives the changes we have made on the private fork of Screenshots: https://github.com/mozilla-services/screenshots-1380616 (and present in the attached XPI)

Comment 19

[Greg G]

Testing the XPI on nightly I get an "iframe URL does not match expected blank.html" on the POC from https://bugzilla.mozilla.org/show_bug.cgi?id=1380616#c7 now.

Trying to screenshot a normal page with the context menu throws a connection failed error and "TypeError: this.save is undefined" from selector/ui.js:571:7, because I don't have the backend running locally.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

n-i to Julien so he will be aware for beta 10 next week.

Comment 21

[Ian Bicking (:ianbicking)]

Attachment: Proposed XPI with security changes and correct backend

This XPI is like the last, but configured to save to screenshots.firefox.com

Comment 22

[Wladimir Palant]

Comment on attachment 8886355 [details] [diff] [review]
security-fixes.patch

Review of attachment 8886355 [details] [diff] [review]:
-----------------------------------------------------------------

From the patch it looks like all the issues have been fixed. Two minor comments below.

::: addon/webextension/clipboard.js
@@ +26,5 @@
> +        if (!copied) {
> +          catcher.unhandled(new Error("Clipboard copy failed"));
> +        }
> +        doc.body.removeChild(el);
> +        document.body.removeChild(element);

I'd suggest removing the frame before throwing the exception about a failed copy, otherwise this frame will stay around forever. In fact, I'd probably even put this removal inside a finally clause to make sure it definitely runs.

::: addon/webextension/onboarding/slides.js
@@ +51,4 @@
>          doc.documentElement.lang = browser.i18n.getMessage("@@ui_locale");
>          localizeText(doc);
>          activateSlide(doc);
> +        iframe.onload = null;

This seems inconsistent. In clipboard.js to load handler is removed immediately, here only after all the other code had a chance to run and fail - so the load handler might still run multiple times if some of the code above throws an exception.

Comment 23

[April King [:April]]

> Actually, that is even more problematic than I thought. It seems that this feature will happily create screenshots of third-party frames. So there you have your data leak - the page can load http://192.168.0.1/ in a frame, make a screenshot of it and get a look at the UI of my home router (similarly for some website on a company intranet). Luckily, major sites like Google or Facebook don't allow framing, otherwise one could deanonymize users this way.

About 89% of the top million sites allow framing. Whether that's a problem for them or not is hard to say, but it's probably safe to say that there are quite a few sites in there that this would be a problem for. I would be surprised if _any_ home router forbids framing -- mine certainly doesn't.

Comment 24

[Ian Bicking (:ianbicking)]

We're getting a report from QA that the clipboard change does not work on Linux or Windows: https://github.com/mozilla-services/screenshots/issues/3130

(I'm a little unclear if they tested Mac, it worked for me during development on Mac)

Comment 25

[Ian Bicking (:ianbicking)]

We're going to go ahead despite the intermittent clipboard regressions we've seen. All this work (including the improvements suggested in Comment 22) is in 10.6.0, tracked in Bug 1381132

And thank you very much Wladimir for finding and explaining the issue so well! It's been very helpful

Comment 26

[Julien Cristau [:jcristau]]

Fixed in bug 1381132

Comment 27

[Ian Bicking (:ianbicking)]

We went through some more review and changes, so the fixes ended up landing in Screenshots 10.8.0. These are on Nightly, and have been uplifted into beta and will be in the July 25th beta build.

Comment 28

[Dylan Hardison [:dylan] (he/him)]

changing milestone format ahead of moving this to Firefox product.

Comment 29

[Wladimir Palant]

Firefox 56 has been released, can this bug be made public?

Comment 30

[Chris Karlof [:ckarlof]]

Dan, can this be made public?

Comment 31

[Daniel Veditz [:dveditz]]

Since we've only got ~30% of our users upgraded to 56 so far I'd prefer to wait until next week before revealing how to hack users with this trick.

Comment 32

[Kris Maglione [:kmag]]

I believe most users of older versions should have received an updated version of Screenshots by now. Can someone from the Screenshots team confirm?

Comment 33

[Ian Bicking (:ianbicking)]

Yes, all of these fixes were shipped in Firefox 55 beta, when Screenshots was still pref'd off by default, and the only way to have an active version of Screenshots without the fix would be to pref on Screenshots manually in about:config in a vulnerable version (which would include Firefox 54, where an even earlier version of Screenshots shipped pref'd off).