Closed Bug 1647078 (CVE-2020-15647) Opened 5 years ago Closed 5 years ago

content:// provider allows websites to remotely steal any file from the device if the path is known

Categories

(Firefox for Android Graveyard :: General, defect, P1)

Product:
Firefox for Android Graveyard
Component:
General
Type:
defect

Tracking

(firefox-esr6878+ verified)

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
firefox-esr68 78+ verified

People

(Reporter: kanytu, Assigned: agi)

References

Details

(Keywords: csectype-sop, reporter-external, sec-critical, Whiteboard: [reporter-external] [client-bounty-form] [verif?][sec-survey])

Attachments

(9 files)

attachments.zip
4.62 MB, application/zip
Details
Bug 1647078 - Load content:// URIs from intents as file://.
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr68+
dveditz
: sec-approval+
Details | Review
Bug 1647078 - Never handle content:// channels from Web.
47 bytes, text/x-phabricator-request
dveditz
: sec-approval+
Details | Review
Bug 1647078 - Use random file names when copying a content:// URI.
47 bytes, text/x-phabricator-request
dveditz
: sec-approval+
Details | Review
Bug 1647078 - Always copy content:// files.
47 bytes, text/x-phabricator-request
dveditz
: sec-approval+
Details | Review
Bug 1647078 - Restrict content:// paths to external storage.
47 bytes, text/x-phabricator-request
dveditz
: sec-approval+
Details | Review
Bug 1647078 - Don't load Web-originated intents that open in the browser.
47 bytes, text/x-phabricator-request
dveditz
: sec-approval+
Details | Review
Bug 1647078 - Fix unit tests and checkstyle. a=RyanVM
47 bytes, text/x-phabricator-request
Details | Review
advisory.txt
264 bytes, text/plain
Details
Attached file attachments.zip —

The following report is in Markdown format, consider using a reader for better experience.

Environment

  • Device: HTC m8
  • OS version: Android 9
  • Package name: org.mozilla.firefox
  • App version: 68.9.0 (versionCode 2015701353) (stable channel)

Proof of concept

Pre-conditions:

  • Firefox installed
  • Python3 installed
  • ngrok (optional for easy tunneling)

Steps:

  • Start the server.py (see attachments) using the following command:

    $ python3 server.py <WEBSITE> <PORT_TO_LISTEN>

    The <WEBSITE> is the URL that is going to be opened in the device without scheme (Examples: 192.1.1.1, myDomain.com, 192.6.1.3:8080).

    The <PORT> is the port for the server

  • Open the website above in Firefox for Android

Result

  • A javascript alert is shown with the contents of the cookies database

Expected result

  • No private file should be accessed

Detailed explanation

When the webiste is opened in Firefox, a webpage is served that contains a <iframe> that downloads a file named profile.ini to /sdcard/Download/profile.ini and opens it after a short delay using this href:

android-app://org.mozilla.firefox/content/org.mozilla.firefox.fileprovider/root/sdcard/Download/profiles.ini#Intent;type=text/html;end

This Intent orders Firefox to open the URI content://org.mozilla.firefox.fileprovider/root/sdcard/Download/profiles.ini. Firefox will download this content:// URI to /data/data/org.mozilla.firefox/cache/contentUri while rendering it. The name is maintained so a new file called /data/data/org.mozilla.firefox/cache/contentUri/profiles.ini has been now created.

The contents of profile.ini can be seen in server.py (method get_profiles_html) but basically it's a HTML page that contains an <iframe> that loads another content:// URI and trigger a javascript function after a while (more info bellow).

The URI loaded is:

content://org.mozilla.firefox.fileprovider/data/user/0/org.mozilla.firefox/files/mozilla/profiles.ini

This provider is misconfigured and contains the following entry in its XML configuration file (provider_paths.xml):

<paths xmlns:android="http://schemas.android.com/apk/res/android">
    <root-path name="root" path="."/>
</paths>

Firefox is using root-path with path . which essentially allows all files to be resolved by this provider. This allows private files from Firefox to be accessed.

The logic to download the file from a content:// URI is rewriting the file, so the contents of profile.ini (the current page loaded) were now overriden and contains the values of /data/user/0/org.mozilla.firefox/files/mozilla/profiles.ini.

After the above <iframe> is loaded, the javascript function triggers. This javascript reads the contents of the current page (this bypass CORS protections because the file is the same). The current page is now the profiles.ini file from Firefox. This file contains the path to the cookies directory. After the path has been retrieved from the file, it repeats the process again but for /data/user/0/org.mozilla.firefox/files/mozilla/<PATH>/cookies.sqlite.

**Resuming: **

This is a combination of multiple misconfigurations/flawed logic:

  • Firefox allows private files to be opened in it
  • Content provider from Firefox allows private file inclusion
  • Files downloaded from content providers with the same name are downloaded to the same place.

Vulnerable code

  • res/xml/provider_paths.xml
<root-path name="root" path="."/>

Remediation

Firefox should not allow private files to be accessed by it's provider. A suggested fix here is to use instead:

<root-path name="root" path="/storage/"/>

Firefox should also not override the contents from contentUri otherwise it'll be possible to override the contents of a file and retrieve it afterwards.

Attachments

  • server.py

    Python3 server containg the full proof of concept.

  • video.mp4

    A video showing the exploit in action.

Impact

  • Remote file steal of cookies or any other file if the attacker knows the path which is easy for common applications.
Flags: sec-bounty?

Stefan/snorp, can one of you (find someone to) investigate?

Group: firefox-core-security → mobile-core-security
Type: task → defect
Component: Security → General
Flags: needinfo?(snorp)
Flags: needinfo?(sarentz)
Product: Firefox → Firefox for Android
Flags: needinfo?(sarentz)

Adding Petru too. The suggested remediation looks good, but I don't know if that will regress anything else.

Priority: -- → P1

Thanks Stefan!
Based on the comment here - https://hg.mozilla.org/mozreview/gecko/rev/703dce7078d2296c209dd0b2e5aabf1790c76361#index_header we needed the FileProvider to allow access to the whole file system as the user has the possibility to set the downloads folder at any location.
So using just <root-path name="root" path="/storage/"/> might indeed break some things.
Will investigate a bit more.

Hey Firefox team,

Given the last comment, the user shouldn't be able to set the downloads folder in the internal folder of the application. No file manager will have access to /data/.. unless the device is rooted, so I still think the suggested approach fixes the issue.

However, I need to enforce that this is only part of the fix. It is important that downloads to /contentUri/ are not replaced and a new file is created even if the name is the same. Otherwise, it will still be possible to steal any file from the device that is saved in /storage/.

Thank you,
@kanytu

Hi kanytu, thank you very much for this report and also for your collaboration towards a good fix for this issue.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: csectype-sop, sec-critical
Summary: A misconfiguration in a content provider of Firefox for Android is allowing websites to remotely steal any file from the device if the path is known. In this report, I demonstrate how it's possible to remotely steal all cookies from a user. → content:// provider allows websites to remotely steal any file from the device if the path is known

so what is this content:// scheme? it's like file:// but worse, and none of the special-cased code that protects file:// from web content. what other non-web internal schemes does Fennec support and expose to web content?

Flags: needinfo?(petru.lingurar)

Is this a possible solution: disable access to content:// URLs from web content?

Yeah why is content:// exposed to web content? I don't think it should? I'm investigating.

Hey again,

I just want to clarify the last comments. The Web has not access to the content:// Uri. What is happening in this exploit, is the usage of android-app scheme to force Firefox to open a content:// Uri webpage.

As seen in your intent-filter:

 <intent-filter>
	 <action android:name="android.intent.action.VIEW"/>
   <category android:name="android.intent.category.BROWSABLE"/>
   <category android:name="android.intent.category.DEFAULT"/>
   <data android:scheme="file"/>
   <data android:scheme="http"/>
   <data android:scheme="https"/>
   <data android:scheme="content"/>
   <data android:mimeType="text/html"/>
   <data android:mimeType="text/plain"/>
   <data android:mimeType="application/xhtml+xml"/>
   <data android:mimeType="image/svg+xml"/>
 </intent-filter>

Firefox supports the content schema. And is that schema I'm sending in this javascript code:

window.location.href = 'android-app://org.mozilla.firefox/content/org.mozilla.firefox.fileprovider/root/sdcard/Download/cookies.sqlite#Intent;type=text/html;end';

This forces Firefox to process an android-app intent (more info here), which will open an HTML page that was just downloaded (by the same page.). So the exploit is local at this point. I just forced Firefox to open the newly downloaded file.

Removing that support for content:// will make Firefox unable to open local HTML files (e.g Open with... )

Let me know if I can help you clarifying the attack vector,

Thanks

@kanytu

kanytu, thanks for reporting this.

I tried to load this html from a local http server in Fennec

<html>
<body>
  <iframe src="content://org.mozilla.firefox.fileprovider/data/local/tmp/test.txt"></iframe>
</body>
</html>

and I got a crash, so at least in some form web content does have access to a content:// URL, which I think it's wrong, but it might be unrelated to this.

Hum I see,

Indeed that shouldn't happen, however the website wont have access to that Iframe because the origin is not the same, so that doesn't seem much of a security problem to me. But that still shows that Firefox tried to download that content uri file to its internal directory. The crash might have been because the URI is correctly generated.

That being said, that doesn't seem related to this attack because the intent redirection turns the vector from remote to local.

FWIW I am not against disabling content:// for Fennec 68.11.

kanytu, do you think changing the root path to /storage/ and disabling content:// scheme from the VIEW intent would be enough to stop this attack?

Flags: needinfo?(kanytu)

Yes. It will. However won't be enough for a local attack with a malicious app I guess. I think an application without storage permissions will still be able to extract files from the device with this exploit by sending an explicit intent to Firefox following the same strategy.

Flags: needinfo?(kanytu)

The above comment is valid only if you don't address the replacement problem in content URI downloads.

If you address that, there is no way this attack can be achieved.

I think this regression was introduced by https://bugzilla.mozilla.org/show_bug.cgi?id=1499618

which in turn seemed to be a regression from https://bugzilla.mozilla.org/show_bug.cgi?id=1406903

which is "cannot view files using content:// scheme"

Given the short timeframe we have towards Fennec 68.11 I want to suggest three options:

  • We remove support for content://. Not just from the manifest and through the VIEW intent, but also from the code introduced by 1406903 and 1499618

  • We remove just the code introduced by bug 1499618 since that is probably the culprit here, trying to be smart about content urls, and doing the actual file copy to a temporary file which is then served.

  • We fix this properly. Keeping the existing functionality but putting proper URL santization and whatever else needed on the handling of content:// URIs

Considering the future of Fennec, my personal preference would be the first option. But it may also be the most risky one since it may touch a lot of code. Although mostly removal probably.

Let's try to make a decision soon. It is not entirely clear what path to take.

I don't love the idea of removing content: URIs globally. This is the modern way of passing data between apps. Android 7.0 switched to content: being the way to reference local files. https://developer.android.com/about/versions/nougat/android-7.0-changes#sharing-files

Bug 1375035 provides a fairly common example of how content: uris are used in Android. Passing an attachment from email apps.

JanH could be a good person to talk to about this.

Removing content: globally probably not , but we should restrict it severely, similar to file:, right now it looks like it's completely unrestricted.

Looks like Fenix doesn't support this case FWIW, so unless we plan to implement it soon this functionality will go away anyway.

I think what we should do here is reject any attempt to load from content:// URIs from in-content, leaving top-level as the only way to load a content:// URIs. So top-level HTML/images etc will still load in Fennec but all sub-resources will be broken.

Flags: needinfo?(snorp)

Apologies if this is a dumb question, I don't normally work on our mobile apps. But why is web content allowed to navigate to android-app: protocol URIs? Is that required for web compat, even without user interaction? And even if we do need it, can we at least block the ones pointing to our own app's content: URL provider?

This change intercepts intents that try to load a content:// URI, copy the
content to a file and then open that file:// in the browser.

Assignee: nobody → agi
Status: NEW → ASSIGNED

This patch forbids loading content:// URIs coming from web content. If a web
page tries to load a content:// url in a sub-resource (like an image, script or
iframe) the browser will return an unknown protocol error.

When copying a content:// URI to a temp file we will use a random file name
instead of the actual file name of the resource to avoid name clashing.

This patch makes it so we never attempt to load a content:// URI by guessing
it's file location, and instead always open a copy.

(In reply to :Gijs (he/him) from comment #22)

Apologies if this is a dumb question, I don't normally work on our mobile apps. But why is web content allowed to navigate to android-app: protocol URIs? Is that required for web compat, even without user interaction? And even if we do need it, can we at least block the ones pointing to our own app's content: URL provider?

android-app: links are the way web pages open an app instead of a web page when navigating to them. We can definitely restrict them to user-interaction-only, and I think Fenix does that already.

Also you're correct into thinking that android-app: should never open Firefox itself, just external apps (and we should probably check that for Fenix?). Although the same attack can be carried over by a local app on the device, which is still pretty bad.

The patches that I've attached to this add some hardening of the content:// handling in Fennec. We don't have to take them all, we can also just disable the content:// protocol outright by just taking this one (plus not responding to the content:// protocol at all in the manifest).

These are the steps that the above patches take:

  • The file name is a secure-random UUID, so name clashing or resource spoofing is not possible anymore
  • We always open a content:// URI to a temporary file, never in the actual location, so that the page cannot request additional resource from protected folders
  • We only accept content:// URIs from intents, and we immediately translate them to the copied-over file:// URL. This ensures that sub-resources cannot load content:// URIs (like scripts, images, iframes). This also means that loading html files only works for plain HTML, all subresources won't be loaded. This is not actually a big loss, since on modern android versions this is basically what happens anyway.
  • We double check that we're not trying to load a content:// URI in the external protocol handler, and if that happens we just reject the load without asking Fennec.

TL;DR After these changes the user can still open attachments from other apps, including HTML files but none of the attack vectors are present anymore. Subresources like images, scripts and iframes from HTML files won't load at all.

One thing I haven't looked at yet is whether an app can still trick Fennec into producing a content:// intent for internal data.

The way I see it, I don't really think this ^^ is a possibility anymore.
After the patches here an app could still start fennec with a content:// or file Intent data (while websites could not anymore) to open a specific file. It could even be an internal storage file.
It could even be the html disguised as profile.ini from here. And we needed this behavior for relatively common usecases like loading/opening device files with Fennec.
I think previously content:// Intents would've probably even worked for overwriting contentUri files. Now with random filenames this isn't possible anymore.
The biggest find in this report is though how through nested htmls(iframes) and by leveraging the overwriting we did in contentUri directory one cleverly designed script could overwrite with and then most importantly extract that data of private files.
After the changes from the patches here to not handle content:// uris requests from the web I don't see any possibility of extracting the data.

Flags: needinfo?(petru.lingurar)

I see that the Content Provider configuration XML was not addressed. It is still exposing the root folder of the device. This can potentially lead to security issues. Is this expected?

(In reply to Daniel Veditz [:dveditz] from comment #6)

so what is this content:// scheme? it's like file:// but worse, and none of the special-cased code that protects file:// from web content. what other non-web internal schemes does Fennec support and expose to web content?

Actually is should be like file:// but better. After Android 7.0 exposing files through the file:// scheme is discouraged (Android changelog).
The content scheme is of the essence here because of how that is handled inside the app: Because it's real source is not known, may not even be a resource on disk, we cache that as a File on disk in the contentUri directory in app's internal storage and then open that inside of Fennec.
If another request is made to load another content uri which would resolve to the same File name the already existing one in the contentUri private directory would get overwritten. And if someone was to already have a reference to that initial File it would get the new content.

(In reply to kanytu from comment #32)

I see that the Content Provider configuration XML was not addressed. It is still exposing the root folder of the device. This can potentially lead to security issues. Is this expected?

I think this is the expected behavior.
Until now there was not a clear indication of how private data could be extracted from Fennec.
It can open any files, yes, but only for the user's eyes.
Only with your report we got a scenario in which a rogue website could extract private Fennec data.

Petru, what would regress if we change the root folder to the suggested /storage/ ? I don't have a good feeling about this . there, which I assume means the root of Firefox. If we don't have a good reason for this, then I would suggest to change it to /storage/?

Flags: needinfo?(petru.lingurar)

(In reply to Stefan Arentz [:st3fan] from comment #35)

Petru, what would regress if we change the root folder to the suggested /storage/ ? I don't have a good feeling about this . there, which I assume means the root of Firefox. If we don't have a good reason for this, then I would suggest to change it to /storage/?

My understanding is that will break opening Downloads for people that moved their download folder in a non-standard location. From what I can see this is only an about:config option?

(In reply to kanytu from comment #32)

I see that the Content Provider configuration XML was not addressed. It is still exposing the root folder of the device. This can potentially lead to security issues. Is this expected?

That's what I meant with Comment 30, I'm looking into that right now :)

(In reply to Agi Sferro | :agi | ⏰ PST | he/him from comment #36)

(In reply to Stefan Arentz [:st3fan] from comment #35)

Petru, what would regress if we change the root folder to the suggested /storage/ ? I don't have a good feeling about this . there, which I assume means the root of Firefox. If we don't have a good reason for this, then I would suggest to change it to /storage/?

My understanding is that will break opening Downloads for people that moved their download folder in a non-standard location. From what I can see this is only an about:config option?

actually it would only break it for people who moved their Download folder to an internal Firefox folder? that seems pretty unusual.

That's what I said in https://bugzilla.mozilla.org/show_bug.cgi?id=1647078#c4

No file manager would give the option to choose an internal folder. For removable sdcards, their path always start with /storage/ too.

(In reply to Stefan Arentz [:st3fan] from comment #35)

Petru, what would regress if we change the root folder to the suggested /storage/ ? I don't have a good feeling about this . there, which I assume means the root of Firefox. If we don't have a good reason for this, then I would suggest to change it to /storage/?

root-path is actually the root of the device (and from what I can see it's an undocumented option). So it's actually worse than you think.

(In reply to kanytu from comment #39)

That's what I said in https://bugzilla.mozilla.org/show_bug.cgi?id=1647078#c4

No file manager would give the option to choose an internal folder. For removable sdcards, their path always start with /storage/ too.

oh I see, sorry I'm still processing all of this, thanks! The user can still type the folder out in about:config even if they don't have access to it but yeah, not really a use case.

My understanding is the same as Agi's.
There is a very small possibility that some users set (through about:config which I wasn't able to, we can test more if needed) their downloads folder to be in some other location that the external storage - what normal users see as storage - device or sdcard.

<root-path name="root" path="."/> normal users, not even root users could use fennec's internal storage + any external storage mediums.
<root-path name="root" path="/storage/"/> would mean any external storage - device memory + sdcard, USB OTG drives, etc.

Flags: needinfo?(petru.lingurar)

root-path is actually the root of the device (and from what I can see it's an undocumented option). So it's actually worse than you think.

It is not documented because it is known to be commonly misused. You can check the source code for Android's FileProvider here

The root-path element points to File("/") . Similar to a ~. It is literally the root of the device.

Agi do you think we should land all the patches attached to this bug? Can you add Petru as a reviewer and I will find another person to take a look?

Flags: needinfo?(agi)

This is unrelated to this issue but after setting up TOTP to access Phabricator, I can't use any action that requires an TOTP. I always get an "Invalid verification code." message. I've tried changing the device time to PST but it doesn't seem to help either. I'm afraid I might be locked out and lose track of this issue if my session expires.

I'm not sure if that TOTP thing is a known issue but if not, and to avoid being locked out, what can I do continue tracking this issue and not lose the account?

I would try reaching out to the Moz Phabricator folks at https://chat.mozilla.org/#/room/#conduit:mozilla.org and explain the situation.

(In reply to Stefan Arentz [:st3fan] from comment #44)

Agi do you think we should land all the patches attached to this bug? Can you add Petru as a reviewer and I will find another person to take a look?

If we want to keep opening content:// links (like email attachments), then yes. Will add Petru as reviewer.

Also I just noticed while looking into this, that our handling of android-app also allows Bug 1356893 to happen. So I added a fix for that too. E.g. something like this navigates to a file URL (even after the fixes) (substitute org.mozilla.firefox with the app name)

<html>
<body>
  <iframe src="android-app://org.mozilla.firefox/content/org.mozilla.firefox.fileprovider/storage/emulated/0/Download/noscript_data.txt#Intent;type=text/html;end"></iframe>
</body>
</html>

for current Fennec substitute the url to: android-app://org.mozilla.firefox/content/org.mozilla.firefox.fileprovider/root/storage/emulated/0/Download/noscript_data.txt#Intent;type=text/html;end.

Flags: needinfo?(agi)

After we're done fixing this I want to compile a list of checks that we should run on Fenix to make sure we're not doing any of this bad stuff there.

I'm gonna push to try as soon as it reopens (squashing the patches and replacing the commit message).

Diana, can QA pick up the above build and do testing around downloads and opening html file from other apps or storage?

Kevin, can you identify some more concrete tests or areas that we should cover?

Flags: needinfo?(kbrosnan)
Flags: needinfo?(diana.rus)

The patches from Agi look awesome!
We'll have an a lot tighter control over who and how uses the content:// scheme leaving a lot less room for abuse.

Regarding the proposed scenario to test the exploit:
I tried along with Diana to test using the proposed STRs:

but we couldn't get the js alert with the cookies in plain on any device that we tested on.
In fact, based on ngrok web interface we never get past the generateProfiles method.
It may be that there's something needed to be changed in server.py.. will investigate a bit.

Curious if others managed to reproduce this, to better assess it's incidence.

Will let Diana present her conclusions.

Hey @petru,

Regarding your comment, you're trying to test that with or without the fix?

Also, make sure you delete the cookies and profile.ini file from the downloads folder before each test. Otherwise, Fennec will rename the file to another one and the exploit wont work.

One more thing, as per server.py, line 20, don't include a scheme (http://) in your parameter. I'm already concatenating a scheme in it.

I had no troubles triggering the inner iframe but if you do, change them to something like <iframe src="http://NGROK_SERVER/generateProfiles"></iframe>

Hi, I didn't managed to reproduce the issue on Firefox Release 68.9.0 (from playstore) with Sony Xperia Z5 (Android 7), Samsung Galaxy S9 (Android 8) nor with Google Pixel 3 XL (Android 9), I managed to trigger the download but the javascript error wasn't present, please check video and settings.

Note:
On the try build (more exactly the build from Android 4.0 API16+ Release opt )i had the following outcome present after doing the steps from description image of the saved file contents.
After accessing a ngrok.io link the first outcome was that a download of the profiles.ini was triggered and afterwards a redirection to Play Store was present.
The iframe remains open in the browser, URL displays the ngrok.io link.

Flags: needinfo?(diana.rus)

Hi, re-checked on Firefox Release 68.9.0 ,based on Comment55 (no second download of the file neither cookies) with Samsung Galaxy S9 (Android 8) and Google Pixel 3 XL (Android 9) - video

Hey Diana,

I've attached a new server.py. I simplified the logic a log. This now only steals profile.ini which should be enough for the test.

Could you please try with the same steps?

https://drive.google.com/drive/folders/1dBzRgpiEKwqSiLANaQC4-pDUx10vmJdD?usp=sharing

Do we need the exploit code hosted somewhere more permanently? I can put it online on a server if that helps.

Thank you @kanytu!
The new script makes it easy to see how a webpage could extract internal storage files content from Fennec.
https://drive.google.com/file/d/1lOi_fAATehvthAjS-6nwEC3ikOu8Zb49/view?usp=sharing
We can now easily test the fix for this and then that all else work as expected.

Hi, yes as Petru's mentioned, i also reproduce video.

After checking with Samsung Galaxy S9 (Android 8) and Google Pixel 3XL (Android 9) on try build from Comment 63 with same str.
Video-Samsung
Video-Pixel 3 XL
After accessing the ngrok.io link, profiles.ini file starts downloading, the navigation bar contains the ngrok.io link and the "Open with" android notification is displayed.

I am unable to replicate this attack in the new APK. I'm pretty sure this exploit has been eradicated by using random names and restricting he content provider's access to private files.

Agi mentioned the following on Slack

it would be nice if we could get some regression testing around opening attachments from email apps (like html pages) and opening downloads (both in Fennec and in other apps)

Hi, checked with Samsung Galaxy S9 (Android 8) and Google Pixel 3 XL (Android 9) on build from Comment63:

  • moved Firefox Download folder from where initial is, to org.mozilla.firefox same outcome is present as when the Download's folder path is not changed(Comment64) - no crash encountered
    The path where the file is saved "/storage/emulated/0/Downloads/profiles.ini".
  • opened attachments from mail (http, https pages, html page with iframe through custom tab and directly in browser)
  • opened files from file://storage/emulated/0/Download/ (images-png, jpg, html)
    Verified also opening files from download in Fenix, no crashes or other problems encountered at opening of downloaded files..

Comment on attachment 9159562 [details]
Bug 1647078 - Load content:// URIs from intents as file://.

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Experimenting with content:// URLs quickly leads to various exploits. How to construct an exploit to read internal files like cookies might not be immediately obvious, but any skilled researcher could definitely do that. We can squash the patches and add a vague commit message to mitigate these concerns, however I would assume that any ESR patch gets a lot of scrutiny anyway.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which older supported branches are affected by this flaw?: esr only
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: N/A this code is only in ESR68
  • How likely is this patch to cause regressions; how much testing does it need?: The code is involved, so regressions are possible. Worst case scenario is Fennec crashes when opening a download or an intent from a third-party app. We ran QA regression testing on these scenarios.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-crit
  • User impact if declined: Any webpage can easily steal all user data (cookies, passwords, etc)
  • Fix Landed on Version: pending
  • Risk to taking this patch: High
  • Why is the change risky/not risky? (and alternatives if risky): The patch is android-only.
    We have to close a few holes in our content:// protocol handling, also we don't have any baking in any other channel since this is a ESR only bug.

A marginally less risky approach would be to remove the feature outright, that means that users would not be able to open attachments from email in Fennec, which has been a feature since 2018.

  • String or UUID changes made by this patch: none
Attachment #9159562 - Flags: sec-approval?
Attachment #9159562 - Flags: approval-mozilla-esr68?
Attachment #9159563 - Flags: sec-approval?
Attachment #9159564 - Flags: sec-approval?
Attachment #9159565 - Flags: sec-approval?
Attachment #9159742 - Flags: sec-approval?
Attachment #9159814 - Flags: sec-approval?

Comment on attachment 9159562 [details]
Bug 1647078 - Load content:// URIs from intents as file://.

sec-approval+ to land whenever release drivers give you the OK for the branch uplift.

Do we want to also land this on nightly just in case people have forked this code for their own use? Or have we landed lots of ESR-branch-only patches prior to this?

Attachment #9159562 - Flags: sec-approval? → sec-approval+
Attachment #9159814 - Flags: sec-approval? → sec-approval+
Attachment #9159563 - Flags: sec-approval? → sec-approval+
Attachment #9159564 - Flags: sec-approval? → sec-approval+
Attachment #9159742 - Flags: sec-approval? → sec-approval+
Attachment #9159565 - Flags: sec-approval? → sec-approval+

(In reply to Daniel Veditz [:dveditz] from comment #69)

Do we want to also land this on nightly just in case people have forked this code for their own use? Or have we landed lots of ESR-branch-only patches prior to this?

discussed privately, there's no m-c counterpart to this, so nothing to land on nightly.

Hello everyone,

I can confirm the fix landed in the latest Firefox version available in Google Play Store.

I want to take the opportunity to congratulate everyone involved for their transparency and professionalism in dealing with this issue.

I would also like to task if this is going to be part of any security advisory and/or CVE.

Thanks,
@kanytu

@kanytu, we haven't shipped a fixed release yet. Our plan was to build and ship a 68.10.1 release with this fix early this week.

You are right. I forgot to give STORAGE permissions to Firefox before making the test :/
Sorry to bother.

Comment on attachment 9159562 [details]
Bug 1647078 - Load content:// URIs from intents as file://.

Approved for Fennec 68.10.1.

Attachment #9159562 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attached file advisory.txt —

FIREFOX_ESR_68_10_X_RELBRANCH (68.10.1):
https://hg.mozilla.org/releases/mozilla-esr68/rev/ad9ac638aa8f2105e9854169fe0de333e463beb6
https://hg.mozilla.org/releases/mozilla-esr68/rev/c87234a340c14826aabf41afc0da0f9df765392f
https://hg.mozilla.org/releases/mozilla-esr68/rev/003ff248966ae98bddc028882e4c70e00b56ea10
https://hg.mozilla.org/releases/mozilla-esr68/rev/f5b74d48979155c082baf699716290d2f154b9c5
https://hg.mozilla.org/releases/mozilla-esr68/rev/bb07a1c15096474bc00516258e5ec4d622ecc718
https://hg.mozilla.org/releases/mozilla-esr68/rev/7e2374db758b3cfcc74a9f26aea9911418ba711c
https://hg.mozilla.org/releases/mozilla-esr68/rev/69fce10096d5ee43bd6f3a979631d62174d652a7

default (68.11+):
https://hg.mozilla.org/releases/mozilla-esr68/rev/d2dba14d515116e556e6403263df12062fef49e3
https://hg.mozilla.org/releases/mozilla-esr68/rev/0c161b487485ec2f1a0248082e05e3e93b4c3317
https://hg.mozilla.org/releases/mozilla-esr68/rev/bfa52add2ca5094a9bc2485f6266d670f0412372
https://hg.mozilla.org/releases/mozilla-esr68/rev/04a1051357b379b59965d46c1a0266bda38288a3
https://hg.mozilla.org/releases/mozilla-esr68/rev/97ae634f83d1d971384d6bf772cd49988ab20952
https://hg.mozilla.org/releases/mozilla-esr68/rev/f1ee7e600869cc5209278ba1851c2b515a4244be
https://hg.mozilla.org/releases/mozilla-esr68/rev/8fbc6afcebf60d4ddb81f8aa603f0961262355a5

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox-esr68: affected → fixed
Resolution: --- → FIXED
Group: mobile-core-security → core-security-release

Hi, verified as fixed with Google Pixel 3 XL (Android 9)-sends user directly to Google Playstore->Firefox and Samsung Galaxy S9 (Android 8) on Firefox Release 68.10.1 with same outcomes present in Comment 64 and with same scenarios from Comment67.
No javascript message present, empty iframe and no crashes encountered.

Status: RESOLVED → VERIFIED
Flags: sec-bounty? → sec-bounty+

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(agi)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][sec-survey]

Done.

Flags: needinfo?(agi)

Thanks Agi - I looked at the form but wasn't sure what to put in there. Except for one thing: I think static code analysis is probably very tricky for this specific bug. (Or maye you had a different idea about that, i'm curious) - But, there is another type of analysis we could do: audit the AndroidManifest.xml and check for changes in for example intent handlers or supported content types. So that when those change, a patch will get flagged as needing a security review. Cc to :sylvestre

Flags: needinfo?(sledru)

Redirecting to Freddy (he is the one owning this rule)

Flags: needinfo?(sledru) → needinfo?(fbraun)
See Also: → CVE-2020-15649
See Also: → CVE-2020-15650

Thanks! I've added this to the response we have on file for this bug.

Flags: needinfo?(fbraun)
Alias: CVE-2020-15647

Hello again,

I'm planning to disclose this bug in a write-up by November.
I'm willing to share the draft here before publish.

However, I would like to ask if the timing is appropriate given the severity of the issue.

I can omit some details if necessary. Do you think it's okay to disclose this by that time?

Thank you,
kanytu

Flags: needinfo?(dveditz)

Thanks for reaching out.
We're expecting users to have received an update for a while now, feel free to go ahead. We'd be interested to read your blog post, when it's ready. Mostly out of curiosity, though.

Great bug and thanks for checking in, Pedro!

Flags: needinfo?(dveditz)

Hey again team,

The writeup has been published today. You can check it out here:

“Firefox and how a website could steal all of your cookies” by Pedro Oliveira https://medium.com/@kanytu/firefox-and-how-a-website-could-steal-all-of-your-cookies-581fe4648e8d

Once again, thank you
@kanytu

Group: core-security-release
Product: Firefox for Android → Firefox for Android Graveyard
Flags: needinfo?(kbrosnan)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: