Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared URLs can be auto-fetched by native applications without respecting same-origin policy #173

Closed
mgiuca opened this issue Aug 26, 2020 · 10 comments · Fixed by #174
Closed

Comments

@mgiuca
Copy link
Collaborator

mgiuca commented Aug 26, 2020

This public vulnerability report by Pawel Wylecial proposes (with proof of concept) the following attack using Web Share:

  1. A malicious website shares a payload containing a url field whose URL would be inaccessible to the website, e.g. a file URL. For example: navigator.share({url: 'file:///etc/passwd'}).
  2. The user chooses a native application, such as iOS's Mail.app, as the share target.
  3. The target application automatically fetches the contents of the URL (in this example, fetching the contents of the user's password database) and attaches it to the email.
  4. The user sends the email, sending the contents of their private file inadvertently, to a recipient controlled by the user.

A couple of things to note:

  • This isn't specifically a vulnerability in the browser (despite the above report calling it a Safari issue). The browser is just sending a well-known URL, which is effectively the same thing as sending plain text. (The URL itself is not a secret, but it points to a file whose contents is.)
  • The vulnerability is due to apparent misbehaviour of the native app, which is not subject to same-origin policy and is therefore able to fetch any URL it wants, including local files. Native apps should not trust incoming intents (which could be coming from any app), but from the browser side, we have no control over the behaviour of native apps.
  • This isn't very severe, as it doesn't allow the website access to the contents of the file, unless they are also able to convince the user to email it to them, by manually typing in the malicious website's email address.
  • This isn't just a problem with file URLs. It affects any "private" URL, such as URLs located on the user's private network (e.g., router configs), and public URLs that may divulge private information if accessed with the appropriate cookies (e.g., what if the native app had cookies to a website the user stores data on, and was able to access a REST endpoint divulging user data).

Things I don't understand:

  • The blog says this affects both Mail.app and Gmail on iOS. I don't understand if both Mail.app and Gmail are both doing the same behaviour and fetching the contents of the URL and setting it as an attachment, or if this is actually Safari fetching the contents of the URL and attaching it to the system intent that it sends to any application. (I am assuming it's the former, since I don't see why Safari would do the latter.)

Proposed solutions:

A. Do nothing (adding a security and privacy consideration to the spec), taking the approach that native apps should not trust incoming URLs.
B. Do nothing at the spec level, but individual implementations can filter out URLs unilaterally.
C. Add a requirement that the implementation filters out file URLs.
D. Add a requirement that the implementation filters out all URLs that are not same-origin as the sending site.

I think C is "theatre", it isn't going to solve the whole problem, just cover up an obvious example.

I think D is the only proper way to prevent this, but would that limit the utility of the API too severely? (It is highly likely that sites are mostly using this to share same-origin URLs, but we don't have data on this.)

@mgiuca
Copy link
Collaborator Author

mgiuca commented Aug 26, 2020

Would it be possible to tweak D to allow "any URL that can be accessed by the sending site via fetch"? I don't know enough about fetch or CORS to know what this really means.

If the sending site can get the contents of a URL, then it should be safe to send it along to a native app that may fetch its contents.

Potential problems:

  • This may require actually sending an HTTP HEAD request to the URL being shared (we don't want to wait for that).
  • Some URLs can only be fetched without credentials. We'd have no way of preventing the native app from fetching the URL with credentials (as discussed in the report above).
  • How can we prevent access to local network ports? Surely fetch has some mechanism to prevent websites from reading http://10.1.1.1.

@marcoscaceres
Copy link
Member

cc'ing @whsieh and @hober

@marcoscaceres
Copy link
Member

I think D is the only proper way to prevent this, but would that limit the utility of the API too severely? (It is highly likely that sites are mostly using this to share same-origin URLs, but we don't have data on this.)

This might break sites like twitter, which routes/redirect everything through "https://t.co/".

@ericlaw1979
Copy link

Surely fetch has some mechanism to prevent websites from reading http://10.1.1.1.

Not today, no, but there is CORS-for-RFC1918 work afoot in Chromium which starts down that path.

@tomayac
Copy link
Contributor

tomayac commented Aug 26, 2020

  • This isn't specifically a vulnerability in the browser (despite the above report calling it a Safari issue). The browser is just sending a well-known URL, which is effectively the same thing as sending plain text. (The URL itself is not a secret, but it points to a file whose contents is.)

I just manually sent a test account of mine an Apple Message containing the plain text file:///etc/passwd. On macOS, it got converted into a link that pointed to the particular local file system of both the recipient and the sender. On iOS, the same message appears as a literal string for both sender and recipient. Sending a message with a URL like https://www.google.com/ causes the recipient side to fetch meta data like the favicon.

Using the demo and sharing to Message causes the actual file to be sent on both macOS and iOS.

Screen Shot 2020-08-26 at 09 11 10

  • This isn't very severe, as it doesn't allow the website access to the contents of the file, unless they are also able to convince the user to email it to them, by manually typing in the malicious website's email address.

I would argue it is severe, especially since users have been trained to ignore "weird" attachments like smime.p7s.

Screen Shot 2020-08-26 at 09 15 47

The attachment looks harmless enough for some users to fall for it, especially since it is trivial to "hide" it below the fold, as demonstrated in the demo.

  • The blog says this affects both Mail.app and Gmail on iOS. I don't understand if both Mail.app and Gmail are both doing the same behaviour and fetching the contents of the URL and setting it as an attachment, or if this is actually Safari fetching the contents of the URL and attaching it to the system intent that it sends to any application. (I am assuming it's the former, since I don't see why Safari would do the latter.)

There is some magic going on, since Gmail even renames the file, whereas Messages uses the original file name.

Proposed solutions:

A. Do nothing (adding a security and privacy consideration to the spec), taking the approach that native apps should not trust incoming URLs.

This would be a solution, but not a great one.

B. Do nothing at the spec level, but individual implementations can filter out URLs unilaterally.

This would be a solution, but not a great one.

C. Add a requirement that the implementation filters out file URLs.

C seems like the right solution, since the sender's file:///foo is not a URL they expect the recipient to be able to access. The Web Share spec should not accept relative-url-string for url, which includes file://, but should probably limit the schemes to be just http and https.

D. Add a requirement that the implementation filters out all URLs that are not same-origin as the sending site.

This seems overly strict, since it would kill off-canonical sharing as well, apart from redirects, as pointed out in #173 (comment).

I think C is "theatre", it isn't going to solve the whole problem, just cover up an obvious example.

Not sure I follow. Private URLs (from your original post) are your responsibility to protect. Local file:// URLs are something different.

I think D is the only proper way to prevent this, but would that limit the utility of the API too severely? (It is highly likely that sites are mostly using this to share same-origin URLs, but we don't have data on this.)

As outlined above, this is overly strict in my opinion.

@marcoscaceres
Copy link
Member

Agree, it's bad. Messages, for instance, doesn't show a preview for "/etc/passwd".

This is WebKit's patch for this:
https://trac.webkit.org/changeset/266151/webkit

Restricting to http(s) might be a good start here. We are still discussing internally within Mozilla what me might do. Will report back once we reach consensus.

@snan
Copy link

snan commented Aug 26, 2020

Option five is to scrap web share.
I can't even with all these standards😭

@martinthomson
Copy link
Member

So there are more problems here than it would seem. The idea that we restrict this to just web origins fails for the same reason that restricting to same-origin fails. Applications that fetch content follow redirects. While a browser might have safeguards in place that prevent redirects from https:// to file://, those applications might not. If the Messages app follows a 301 to a file:// URL, then it is trivial to work around the fix that Apple proposes.

This is, of course, the same workaround to a same-origin restriction (option D). If we only allow a site to share same-origin URLs, it can add a redirect to anywhere. As applications are capable of following redirects (Marcos found that many do, including those used in the proof of concept), this means option D isn't as useful as it seems.

In the extreme case, the same spoofing can be performed for in-browser sharing UX that only shares the pages you are actively visiting. The site can use User-Agent provide HTML to web browsers and redirects for vulnerable requesters.

The basic problem is that there are two reasons you might "share" with an application:

  • The application provides a communication channel, through which you want to pass a reference.

  • The application consumes the URL and the referenced content.

In the first case, having the application retrieve content (using ambient authority, which means access control based on where the application runs rather than anything like cookies) presents a risk to the information that is retrieved. That's this attack.

In the second case, if the application just wants to take the content and display it locally, that is fine. It's not inappropriately exploiting its location.

The only place where corrective action might be taken is the operating system and it can't reasonably distinguish between these uses. There is a small role perhaps: if the OS is the one providing the machinery that fetches content, including following of redirects, then it might apply some of the same protections browsers provide for redirects and navigations. For instance, Firefox (and I'm sure others) won't follow a redirect or link from https:// to file:// or chrome://. Add CORS-1918 and you gain a tiny bit more protection (though this too is not perfect as it assumes incorrectly that private use address ranges are the only ones that authorize based on source address, this is only mostly true).

Ultimately, I'm forced to agree with Matt that the bulk of the responsibility lies with the OS and applications. More so because that is where corrective action has to occur unless we think that sharing links is fundamentally a bad idea. There is little a browser can do to prevent this without completely removing the ability to share URLs entirely, possibly even from browser UX (with redirects, see above). That seems unnecessary. Personally, I think that removing sharing would do far more harm than this attack could. That doesn't mean browsers are off the hook entirely; we're implicated in the crime as accessories.

It probably makes sense to apply the simple mitigations anyway. Limit this to resources that we might navigate to. However, more broadly we need to look more carefully at how user interaction is involved here. Applications that retrieve content and send it somewhere are the main problem, and how those present information is important. If this does end up with sharing something and that is truly what the user intended, I don't see that we should stand in the way.

(Maybe the data leak is of interest to enterprise security folks concerned about data exfiltration and they might object, but OS-level controls for those people seem like the best remedy there.)

That still means understanding how people react to this sort of accidental oversharing. That might depend on how applications present what is being shared before sharing it. For instance, people could easily miss the fact that the cat picture is somehow a text file, even when presented with a file icon rather than an image preview.

@snan
Copy link

snan commented Aug 27, 2020

More so because that is where corrective action has to occur unless we think that sharing links is fundamentally a bad idea.

Sharing links is a good idea. (If sharing links was an unappealing idea, this attack vector wouldn't be a problem because people would just not use the share feature.)

But it's a question of degree of

  • how much the link sharing is done programmatically by code written & unvetted by the user
  • how much of it is done by local, trusted apps, and
  • how much it's done by manually dragging/typing stuff around.

That's a triangle of gradients rather than a boolean question.

Obviously neither of those three corners is fully reachable, because the computer is to some extent "turtles all the way down" or at least till we get down to voltages on wires—we do want (on some level) code to help us share links—but letting that code be pulled and executed from rando servers on the Internet is a bridge too far.

It sounds contradictory to not trust browser devs to design and implement link sharing UI, while also definitely trusting them to correctly do all the vetting, checking and executing for a similar system served remotely.

For instance, people could easily miss the fact that the cat picture is somehow a text file, even when presented with a file icon rather than an image preview.

When you have flow and you're working in an area you've come to believe that you can trust and know, you don't want stop to double-check everything. Ideally you want a workflow so smooth that it's a matter of seconds or less than that to do tedious chores like sharing links. Imagine if typing on a keyboard was so untrustworthy that after hitting every key you had to cautiously look to see that the key was correct, before you could feel OK about hitting the next.

Please don't build a web even more untrustworthy than it is.

@marcoscaceres
Copy link
Member

marcoscaceres commented Aug 27, 2020

Initial patch is up at: #174

It rejects non-http(s) URL.

Given we have the files member, there doesn't seems to be any reason to pass around data: URLs either, right?

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 24, 2020
We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 24, 2020
We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 24, 2020
We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <[email protected]>
Reviewed-by: Glen Robertson <[email protected]>
Auto-Submit: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/master@{#810180}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Sep 24, 2020
We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <[email protected]>
Reviewed-by: Glen Robertson <[email protected]>
Auto-Submit: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/master@{#810180}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 24, 2020
We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <[email protected]>
Reviewed-by: Glen Robertson <[email protected]>
Auto-Submit: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/master@{#810180}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 26, 2020
…nd https, a=testonly

Automatic update from web-platform-tests
Web Share: restrict URL scheme to http and https

We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <[email protected]>
Reviewed-by: Glen Robertson <[email protected]>
Auto-Submit: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/master@{#810180}

--

wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6
wpt-pr: 25755
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 28, 2020
…nd https, a=testonly

Automatic update from web-platform-tests
Web Share: restrict URL scheme to http and https

We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <ericwilligerschromium.org>
Reviewed-by: Glen Robertson <glenrobchromium.org>
Auto-Submit: Eric Willigers <ericwilligerschromium.org>
Cr-Commit-Position: refs/heads/master{#810180}

--

wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6
wpt-pr: 25755

UltraBlame original commit: 527027b783378db7e3c1fcdd908e718799400989
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 28, 2020
…nd https, a=testonly

Automatic update from web-platform-tests
Web Share: restrict URL scheme to http and https

We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <ericwilligerschromium.org>
Reviewed-by: Glen Robertson <glenrobchromium.org>
Auto-Submit: Eric Willigers <ericwilligerschromium.org>
Cr-Commit-Position: refs/heads/master{#810180}

--

wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6
wpt-pr: 25755

UltraBlame original commit: 527027b783378db7e3c1fcdd908e718799400989
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 28, 2020
…nd https, a=testonly

Automatic update from web-platform-tests
Web Share: restrict URL scheme to http and https

We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <ericwilligerschromium.org>
Reviewed-by: Glen Robertson <glenrobchromium.org>
Auto-Submit: Eric Willigers <ericwilligerschromium.org>
Cr-Commit-Position: refs/heads/master{#810180}

--

wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6
wpt-pr: 25755

UltraBlame original commit: 527027b783378db7e3c1fcdd908e718799400989
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Oct 1, 2020
…nd https, a=testonly

Automatic update from web-platform-tests
Web Share: restrict URL scheme to http and https

We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <[email protected]>
Reviewed-by: Glen Robertson <[email protected]>
Auto-Submit: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/master@{#810180}

--

wpt-commits: a28408e23e7cb1e4e8dc070445a51fc2f2d9a4e6
wpt-pr: 25755
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
We now follow the recent spec change limiting the permitted scheme
for shared urls to http and https - see
w3c/web-share#173
w3c/web-share#174
w3c/web-share#177

We make an exception if the page performing the share it itself loaded
from a different scheme (e.g. file) - in that case we allow the same
scheme to be used for the shared url.

Bug: 1131755
Change-Id: I6abf0f9acd40ef79ec49379314e2ef3a81d3467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425977
Commit-Queue: Eric Willigers <[email protected]>
Reviewed-by: Glen Robertson <[email protected]>
Auto-Submit: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/master@{#810180}
GitOrigin-RevId: 060b7f1b2de01048a934bc4aca41973edaf4d12c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants