+torne, can you help find an owner for this?

One easy fix might be to register the content scheme as a local scheme with WebSecurityPolicy::registerURLSchemeAsLocal. I'm not sure if we need special handling elsewhere as well...

Per the conversation in  https://crbug.com/439083 , I thought we did mark `content:` as local in `AwContentRendererClient::RenderThreadStarted`. It's not clear to me why that's not enough to prevent navigation from webby URLs to `content://`.

Charlie might have thoughts?

One option might be to mark `content:` URLs via `registerURLSchemeAsNoAccess`, which would force them into unique origins, thereby blocking DOM access to the iframe. Depending on `content:`'s usage, that might have too many side-effects...

A couple of thoughts here:

1) Why on earth is the download manager's content provider BROWSABLE in the first place? That seems like a bug in DownloadProvider, though it's one that we can't get fixed retroactively..

2) Mike's recollection in #3 is correct, but applies only to WebView. WebView marks content: local, but in webview-specific code.

3) I'm not sure what the use cases for content: in Chrome actually are. WebView needs to support it for Reasons, but I didn't realise it even worked in Chrome at all; I thought BROWSABLE intent filtering only applied to intent URLs for activities, not the content: scheme.

Looks like Grace reassigned this on 659489, so doing the same here.

Comments 3 and 7: Is registering it as local sufficient to prevent access?  I think we want IsWebSafeScheme to return false (in ChildProcessSecurityPolicy), which means we shouldn't calling GrantScheme for it.

Looks like Android Webview is already calling GrantScheme in AwContentBrowserClient::RenderProcessWillLaunch, so it's possible there's no protection there either.  Can someone check whether this bug is exploitable in Android Webview as well?

(Agreed that it seems really bad that content:// can be navigated to by a web page.)

I think for webview it needs to access resources which is content scheme.
For Chrome, a downloaded webpage will have content:// scheme, although file:// scheme can also be used, but content scheme is preferred for sharing with another app.

Comment 15: I understand that Android Webview may depend on the content:// scheme for some things, but does this bug show that it's vulnerable as well?  That is, could a web page loading in a Webview directly access files the user has downloaded?  If so, we may need to find a way to distinguish between that case and the things it depends on, right?

Yes, a webpage in webview can access content scheme downloaded by another webview session.

However, for file scheme, webview has a method setAllowFileAccessFromFileURLs() to block it, by default that setting is set to false.

+sgurun to review for WebView.

I don't do anything involving these URIs.

Is BROWSABLE matter? That is only enforced by Android ActivityManger. I don't see the relationship here.

It seems when we added content:// support, we didn't add any protection.

For download case, we may want to limit it to a single source, so we can add a similar protection as setAllowFileAccessFromFileURLs for content://.

For WebView, we can also add an authority check, https://developer.android.com/reference/android/content/ContentUris.html, if it makes sense.

just using my memory: setAllowFileAccessFromFileURLs (or the equivalent) should already be false for chrome.

if we don't allow content scheme access from Content URLs, does this has any impact on webview? 

does somebody have a easy repro case? When I tried to repro it using XMLHttpRequest   GET, I got:
44 I chromium: [INFO:CONSOLE(16)] "XMLHttpRequest cannot load content://downloads/my_downloads/20. Cross origin requests are only supported for protocol schemes: http, data, chrome, https.", source: http://output.jsbin.com/xelozem (16)

additionally, is it possible for attacker to use file url to access content scheme (vice versa) to bypass setAllowFileAccessFromFileURLs()? 

XHR is subject to same-origin restrictions. Does it work if the content is iframed?

(Sorry I don't have an easy way to test on Android)

yes I can load the content in an iframe, thx

I have a poor man's attack for this where I:
1. use document.location to redirect to a content file:
  document.location='content://downloads/my_downloads/24';
2. in the local file (my_downloads/24), I read another file and put into a form:
 <html>
  <head>
    <script>
      function f() {
        var ifrm = document.getElementById('test');
        var a = ifrm.contentWindow.document.body.textContent;
        document.getElementById('username').value=a;
      }
    </script>
  </head>
  <body onload="f()">
    <iframe id="test" src="content://downloads/my_downloads/23"></iframe>
    <form>
      First name:<br>
      <input id="username" type="text" name="firstname"><br>
    </form>
  </body>
</html>

this works fine in chrome but in WebView it fails with:
10-27 00:30:05.376 26089 26089 I chromium: [INFO:CONSOLE(0)] "Not allowed to load local resource: content://downloads/my_downloads/24", source: http://output.jsbin.com/duledic/4 (0)

so I guess the protection we added to WebView (in comment #3) helps.

In Chrome, trying to access a content:// url from a file:// url fails as below, if I put the script in #31.2 above to a local file

 chromium: [INFO:CONSOLE(6)] "Uncaught SecurityError: Blocked a frame with origin "null" from accessing a frame with origin "content://".  The frame requesting access has a protocol of "file", the frame being accessed has a protocol of "content". Protocols must match.

However, in chrome if I read a local file and use document.location to change it to a content:// first (as given in #31.1) then the attack works of course. 

Trying the same in WebView fails with UNKNOWN_URL_SCHEME.

Deleted: Screenshot_20161027-011252.png 111 KB

	Screenshot_20161027-011252.png 111 KB View Download

I think as a first mitigation we can make the scheme "content" local in chrome. If nobody has an objection, I will try it and then upload a cl for that. 

Thanks, go ahead. Webpage shouldn't be able to navigate to content:// urls at all. Also, can someone explain the current usecase behind content:// urls.

We need to get a fix landed on trunk by tomorrow, preferably tonight, so if no one has other ideas and that will address this issue I don't object - would love for some more security-oriented devs to chime in here though.

Wait, what will this fix ? see first line in c#0.

Chrome for Android allows web sites to automatically download HTML pages to the device and redirect to them
using the content:// scheme. Once in the Content scheme, the Same Origin Policy does not protect a
downloaded HTML from accessing other content files, such as other downloaded files.

I don't know what the current use case for content:// URLs in Chrome is. Is it possible to navigate Chrome to a content:// URL via an intent? If so then it's possible some apps expect to be able to launch a browser for a content:// URL and disallowing this will break them. However, if Chrome doesn't accept these as incoming intents then I can't think of an obvious legitimate use case for permitting them at all..

WebView apps use content: URLs and WebView needs to continue supporting this. It's not clear whether there's actually a security issue here with WebView currently, given that webview already marks the scheme local, unless I'm missing something.

#41 Yes Chrome allows using content:// intents. Note that downloadprovider has a UID check, therefore the file that is accessed has to be downloaded by Chrome.

#40 My understanding is making it local prevents at least the cases I came up with above, i.e. using document.location to redirect to a content file. If we want to restrict further, It think we need something like setfileaccessfromfileurls to make each unique? 

I remember when saving an html, then later on opening it will require Chrome to support content://

creis@: or actually anybody, do you have any attack scenario that I can implement and play with?

There are two important things here:
- Make sure web content can't embed a content:// URL or redirect to it.
- Make sure one content:// URL is considered cross-origin to another content:// URL

Marking it local should be sufficient for Blink, but creis@ should comment on the web-safe scheme portions: it sounds like it's also incorrectly being marked as a web-safe scheme via GrantScheme().

I introduced the content scheme support to Chrome here:
https://chrome-internal-review.googlesource.com/#/c/186392/

Basically after saving a link, when clicking the entry in Downloads app, Chrome need to pop up.

as dcheng said, once I made content local, when I tried to access another content file contents from a content file, I got:

10-27 17:35:20.900 14940 14940 I chromium: [INFO:CONSOLE(6)] "Uncaught SecurityError: Blocked a frame with origin "null" from accessing a frame with origin "null". Protocols, domains, and ports must match.", source: content://downloads/my_downloads/25 (6)

I am uploading a CL for local first. The GrantScheme problem only impacts webview, AFAIK.

- Make sure web content can't embed a content:// URL or redirect to it.

I think this can be done by  blink::WebSecurityPolicy::registerURLSchemeAsLocal(content_scheme);. WebView already seems to do that. https://cs.chromium.org/chromium/src/android_webview/renderer/aw_content_renderer_client.cc?rcl=0&l=78

- - Make sure one content:// URL is considered cross-origin to another content:// URL

I think this can be done by marking content:// as a unique origin (just like data: ?). Are we sure that content:// is not supposed another content:// url in regular usecases ?

The only case Chrome wants to support is downloading an html page and opening it from downloads app.  The url in this case will be mapped to a download Id, and it will sounds very fishy that a content:// tries to access another content:// url since the former won't be able to guess the download Id of the latter.

According to #47, registerURLSchemeAsLocal() should block both things mentioned in #45

Here is what I did for #47. 
Note: Names of the files are pretty meaningless, so don't care.

bullhead:/sdcard/Download # cat reserved.html                                  
 <html>
  <head>
    <script>
      function f() {
        var ifrm = document.getElementById('test');
        var a = ifrm.contentWindow.document.body.textContent;
        document.getElementById('username').value=a;
      }
    </script>
  </head>
  <body onload="f()">
    <iframe id="test" src="content://downloads/my_downloads/26"></iframe>
    <form>
      First name:<br>
      <input id="username" type="text" name="firstname"><br>
    </form>
  </body>
</html>


bullhead:/sdcard/Download # cat rfc2606.html                                   
mypassword is my name


for content://downloads/my_downloads/26  Download provider serves rfc2606.html
for content://downloads/my_downloads/25 Download provider serves reserved.html

Loading 25 using 
adb shell am start -a android.intent.action.VIEW -n com.google.android.apps.chrome/com.google.android.apps.chrome.Main -d content://downloads/my_downloads/25

gives a 
10-27 18:46:15.690 14940 14940 I chromium: [INFO:CONSOLE(6)] "Uncaught SecurityError: Blocked a frame with origin "null" from accessing a frame with origin "null". Protocols, domains, and ports must match.", source: content://downloads/my_downloads/25 (6)


Therefore making content local should be satisfactory for making content origin unique with respect to another.

Piping in re #48 "make sure one content:// URL is considered cross-origin to another content:// URL". Just a bit of landscape:

 - I would want to confirm that kContentScheme is not registered with GURL as a standard scheme --- via AddStandardScheme() in url_util.h. From looking at the code I don't think it is. If it's not registered as a standard scheme, then we ought to get unique origins when converting from a content GURL to an url::Origin(). We are increasingly, but not yet universally, implementing the browser-process origin logic in terms of url::Origin. However, within blink, SecurityOrigin (which is much richer) is used. Note that url::Origin behavior today mostly influences process placement; allow/deny of requests is based on URL-based checks, but there is some interrelation as well (e.g. this determines whether "blob:content://" is seen as well-formed).

 - From looking at the code, I would expect that ChildProcessSecurityPolicy::IsWebSafeScheme() would returns false for the content scheme. That's the safe behavior. This means that navigations to that scheme ought to be denied, via the FilterURL mechanism, unless the requesting renderer process has been given permission via ChildProcessSecurityPolicy::GrantScheme().

- As others have said, the existing GrantScheme() to all android-webview renderers seems problematically broad.  A mechanism more like ChildProcessSecurityPolicyImpl::GrantRequestSpecificFileURL() -- where individual content URLs are granted, e.g. as the embedding app tells its webview to navigate to them -- would be safer, if we can pull it off. Can we?

Overall I would expect the file:// scheme to be a good model for how the content:// scheme should behave.

As one more thing to check, registering `content:` as "local" will prevent one `content://` URL from accessing another iff the "allowFileAccessFromFileURLs" setting is not `true` (see https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Document.cpp?rcl=1477567199&l=5349 in `Document::initSecurityContext()`).

If that setting isn't set, then `content:` can access `content:` (see https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp?rcl=0&l=254 in `SecurityOrigin::canAccess()`. It doesn't look like that's set by default on Android, but it's worth checking to make sure.

#52, WebView has an API to allow access to content urls from WebView, and by default it is enabled:

https://developer.android.com/reference/android/webkit/WebSettings.html#setAllowContentAccess(boolean)

If I understand correctly, what you ask for requires an API change to allow embedder apps to give grants for unique file urls. I don't think we can make this change now because I don't know what it will break at the end.

we can try to do a change in O timeframe. 

If the webview is not currently navigated to a content:// URL, we should be able to block access to content URLs, right? Are http/https pages allowed to have links to content:// URLs?

I'm a bit behind on the discussion here, but there's a test failure on the proposed CL:
https://codereview.chromium.org/2456883002/
org.chromium.chrome.browser.UrlSchemeTest#testContentUrlFromData

Are data URLs or web pages allowed to load content:// URLs inside images?  That's what the test seems to imply, and it may be a problem for fixing this.

I was confused why the content: scheme is allowed through FilterURL at all, and I am thinking it's because we're missing the following line: https://codereview.chromium.org/2456123002/diff/1/chrome/browser/profiles/profile_io_data.cc ? I'm running that through the tryjobs now, I expect it will break the UrlSchemeTests.

I.e., we install a protocol handler, but the static function* that lists the schemes that have handlers does not know about it, so CanRequstURL treat it like "mailto:" and other externally-handled protocols. (* ==> we need a static list of these, so that FilterURL works from all threads)

correct it does cause failures  (locally tested). After applying your change on top of mine, testContentUrlFromFile also starts failing. without it does not and I was thinking why it does not.

Yes, file html with content URL is still ok when registering "content" as "local", but an iframe which contains content URL is not ok.

For example, a file html with content URL in iframe will result in the following error:
"Uncaught SecurityError: Blocked a frame with origin "null" from accessing a frame with origin "null".  The frame requesting access has a protocol of "file", the frame being accessed has a protocol of "content". Protocols must match.

But having a <img src="content://...."> </img> doesn't generate any errors after registering content as local

for #56, data URL should be treated as the same as webpage. The test used to use urls, but later was changed to data URL to avoid flakyness. But we should not allow data URL to access content://

sgurun: does the patch in #57, by itself, mitigate the attack? I'm not set up to test locally.

I expect this to help at the "chrome can be redirected to the content:// url of this download" step.

It ought to suppress using window.location and other script actions to cause any navigation to a content URI (such attempts ought to rewrite to about:blank). browser-initiated navigations (such as typing a content URI into the omnibox) ought to still work.

nick:

If I use omnibox to enter a content: url that refers to another content: url, I see
10-27 21:39:43.656  1781  1781 I chromium: [INFO:CONSOLE(6)] "Uncaught SecurityError: Blocked a frame with origin "content://" from accessing a frame with origin "null".  The frame requesting access has a protocol of "content", the frame being accessed has a protocol of "data". Protocols must match.

is this expected?

#62, with your patch by itself, i got the following error when using a javascript to redirect a page to a content:// url

"Uncaught (in promise) SecurityError: Only secure origins are allowed (see: https://goo.gl/Y0ZkNV).", source: http://null.jsbin.com/runner (26)


It also blocks a file url from embedding a content url image.

However, it doesn't block a content url page from reading an iframe or embedding an image whose url is also an content url.

So looks like we need both, the CL from #57 and Selim's CL to registering content as local

Double check the contents of your html. From the error, it looks like you tried to reach into an <iframe src="data:">. That's always an error, regardless of whether you're using the content scheme or not.

(#66 was in response to #63)

ok it was my html after all. agreed with qinmin's results.

If it is ok, I can update my CL to include nick's changes, and fix the two failing tests (no need to delete, we can use them to make sure file and data uri's can't access to content uri's - qinmin's idea).

I can write another test to make sure content uri's cannot access other content uri's but this probably has to wait until tomorrow.

#64-#65: Agree. qinmin, can you fold https://codereview.chromium.org/2456123002 into yours? I see a 3 failing tests on the android trybot so far that need to be dealt with.

Also we need to make sure that we don't regress the original use case for supporting content:// in clank (which had something to do with opening an mhtml file saved to an sdcard?). Hopefully these just work, as browser-initiated navigations.

re #69-70, I was confused: qinmin is assigned this bug but sgurun@ has a fix in progress. sgurun@, please fold my proposal into yours, thanks!

Since sgurun@ already has a CL to fix this, removing myself as the owner and reassign it to sgurun@

Let's make sure the fix gets into M55 as well.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5eb1cbdd8f2ed45d8bf92630ed170335b983ee87

commit 5eb1cbdd8f2ed45d8bf92630ed170335b983ee87
Author: Selim Gurun <sgurun@chromium.org>
Date: Fri Oct 28 18:45:41 2016

Make the content scheme local

WebContent should not be able to embed or redirect to content scheme.

BUG= 659492 

Review-Url: https://codereview.chromium.org/2456883002
Cr-Commit-Position: refs/heads/master@{#428222}
(cherry picked from commit c7c7daa089804c779c4ebfb2a2b5793be6b47090)

Review URL: https://codereview.chromium.org/2458793003 .

Cr-Commit-Position: refs/branch-heads/2883@{#360}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/5eb1cbdd8f2ed45d8bf92630ed170335b983ee87/chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
[modify] https://crrev.com/5eb1cbdd8f2ed45d8bf92630ed170335b983ee87/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/5eb1cbdd8f2ed45d8bf92630ed170335b983ee87/chrome/renderer/chrome_content_renderer_client.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3661b5725db8002abebe48400ae22073ca66b504

commit 3661b5725db8002abebe48400ae22073ca66b504
Author: Selim Gurun <sgurun@chromium.org>
Date: Fri Oct 28 18:51:56 2016

Make the content scheme local

WebContent should not be able to embed or redirect to content scheme.

BUG= 659492 

Review-Url: https://codereview.chromium.org/2456883002
Cr-Commit-Position: refs/heads/master@{#428222}
(cherry picked from commit c7c7daa089804c779c4ebfb2a2b5793be6b47090)

Review URL: https://codereview.chromium.org/2455133004 .

Cr-Commit-Position: refs/branch-heads/2840@{#794}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/3661b5725db8002abebe48400ae22073ca66b504/chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
[modify] https://crrev.com/3661b5725db8002abebe48400ae22073ca66b504/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/3661b5725db8002abebe48400ae22073ca66b504/chrome/renderer/chrome_content_renderer_client.cc

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This change affects WebView as well as chrome, correct? It seems quite possible that some webview-embedding apps are depending on the lax security of content: URLs and will be broken by tightening it.

I guess we can wait to see what appears to break/what people complain about, since we don't know of any specific cases right now, but for example I wouldn't be surprised if someone embeds a content:// URL from a data: scheme page when the data: URL is generated locally in the app code.

#81: the changes I landed are Chrome only. 

The bug is not fixed yet: The missing pieces:

1. Landing a test. I have been working on it, but I hit some issues and debugging took a while. 
2. Investigating and improving WebView.
  -GrantScheme: Can we find a better mechanism?
  -How big a risk factor the current situation that we give access to content urls by default? How can we improve it?

Not P0 anymore. 

After landing the test, I will spin-off the webview piece in a separate bug CC'ing people here.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ddb80fd8f7bcba05f993b678d858ca687b98870

commit 1ddb80fd8f7bcba05f993b678d858ca687b98870
Author: sgurun <sgurun@chromium.org>
Date: Sat Nov 05 00:37:28 2016

Add test for verifying that iframe contents that are served from content urls cannot be accessed by parent page.

BUG= 659492 

Review-Url: https://codereview.chromium.org/2476823002
Cr-Commit-Position: refs/heads/master@{#430093}

[modify] https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870/chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
[modify] https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870/chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
[add] https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870/chrome/test/data/android/page_with_iframe_as_content_url.html
[add] https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870/chrome/test/data/android/simple_iframe.html

fixed. opened crbug/662612  for investigating WebView.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9827b0296317830eac3ad9920103822f57930525

commit 9827b0296317830eac3ad9920103822f57930525
Author: mathp <mathp@chromium.org>
Date: Sat Nov 05 04:01:13 2016

Revert of Increase test coverage for Content:// urls (patchset #6 id:100001 of https://codereview.chromium.org/2476823002/ )

Reason for revert:
Failures in UrlSchemeTest

https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/36968

Original issue's description:
> Add test for verifying that iframe contents that are served from content urls cannot be accessed by parent page.
>
> BUG= 659492 
>
> Committed: https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870
> Cr-Commit-Position: refs/heads/master@{#430093}

TBR=tedchoc@chromium.org,nick@chromium.org,sgurun@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 659492 

Review-Url: https://codereview.chromium.org/2473153005
Cr-Commit-Position: refs/heads/master@{#430147}

[modify] https://crrev.com/9827b0296317830eac3ad9920103822f57930525/chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
[modify] https://crrev.com/9827b0296317830eac3ad9920103822f57930525/chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
[delete] https://crrev.com/0b0259349e5a2358021cba9d63b1bcbe706a99fc/chrome/test/data/android/page_with_iframe_as_content_url.html
[delete] https://crrev.com/0b0259349e5a2358021cba9d63b1bcbe706a99fc/chrome/test/data/android/simple_iframe.html

relanded the reverted CL after some fixes to address android multidex support in debug bots

https://codereview.chromium.org/2514653003/

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot