Closed Bug 1617928 (CVE-2020-6828) Opened 11 months ago Closed 10 months ago

Firefox for Android - Directory Traversal can lead to network hijacking

Categories

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

defect

Tracking

(firefox-esr6875+ fixed, firefox73 unaffected, firefox74 unaffected, firefox75 unaffected, firefox76 unaffected)

RESOLVED FIXED
Tracking Status
firefox-esr68 75+ fixed
firefox73 --- unaffected
firefox74 --- unaffected
firefox75 --- unaffected
firefox76 --- unaffected

People

(Reporter: gnehsoah, Assigned: petru)

References

Details

(Keywords: csectype-priv-escalation, sec-high, Whiteboard: [reporter-external] [client-bounty-form][adv-esr68.7+])

Attachments

(3 files)

Attached file poc.zip

It has been found the firefox android application accepts Intents from third-parties. When a crafted Intent containing a URI pointing to a custom-defined ContentProvider is sent, the application queries the ContentProvider to fetch files. This allows overwriting files under the private application folder.
By exploiting this vulnerability, it is possible to overwrite /data/data/org.mozilla.firefox/files/mozilla/profiles.ini and put a user.js file into the user's directory which can lead to network hijacking.

Vulnerability in this method:
org.mozilla.gecko.util.ContentUriUtils.getTempFilePathFromContentUri

Firefox Version: 68.5.0

Steps to reproduce:

  1. see the screen record
Flags: sec-bounty?

Stefan, Snorp, can you please take a look at this? I can't really say how viable an attack through crafted intents is, but overwriting arbitrary prefs/files in the app directory is almost as bad as it can get.

Thanks!

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

Ugh. Yup, this is pretty bad.

It looks like in order to fix, we should just use the leaf file name here: https://searchfox.org/mozilla-central/rev/b2ccce862ef38d0d150fcac2b597f7f20091a0c7/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java#321

If we don't do this, the filename can look like "../../files/mozilla/profiles.ini", which causes us to overwrite our own profiles.ini, etc. We should treat it as "profiles.ini" instead.

Flags: needinfo?(snorp)
Assignee: nobody → snorp

Petru, do you have time to write a patch for this? See comment #3.

Flags: needinfo?(petru.lingurar)

This affects Fennec and Fenix, doesn't it?

We ought to patch both.

Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form]

Sebastian tells me this only affects Fennec.
From what I understand, we want to roll out fenix gradually to existing beta users, which means we can't easily ship a security update without delaying that. But updating to would fix this problem for beta population over the course of the next ~2 weeks.
This would still leave fennec release users affected until we start the roll out in early April (~5 weeks from now).

Maybe we could do the fenix roll-out for beta and fix fennec-release directly after beta has gone to 100% (i.e., after those two weeks)?
I'd still very much like to hear what st3fan and Dan think.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #4)

Petru, do you have time to write a patch for this? See comment #3.

Sure!
This is interesting...

Flags: needinfo?(petru.lingurar)
Flags: needinfo?(sarentz)
Priority: -- → P1
Assignee: snorp → petru.lingurar
Status: UNCONFIRMED → NEW
Ever confirmed: true

Marking this a P1 - lets try to ship this in the next release.

For loading a file exposed through a provider Fennec will first copy the file
in it's cache folder (internal storage).
Tricking Fennec into thinking the file name should contain forward slashes will
result in saving the file to a different than intended location potentially
overwriting important application data.

To mitigate this attack vector we'll always check for forward slashes in the
filename and if so always keep just the leaf.

James, I see FileUtils still exists in m-c.
If it needs this change also I can file a bug and post a similar patch for it.

Flags: needinfo?(snorp)

(In reply to Petru-Mugurel Lingurar[:petru] from comment #10)

James, I see FileUtils still exists in m-c.
If it needs this change also I can file a bug and post a similar patch for it.

That would be great. I could see us trying to use it in the future to handle content://.

Flags: needinfo?(snorp)
Blocks: 1619997

Sebastian tells me this only affects Fennec.

The vuln combination, maybe, because we block it at the intents end. We should fix FileUtils for both just in case there's another or future way to get there in other GeckoView products (what snorp said in comment 11).

Does shipping Focus respond to or ignore those intents?

Flags: needinfo?(dveditz) → needinfo?(snorp)

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

Sebastian tells me this only affects Fennec.

The vuln combination, maybe, because we block it at the intents end. We should fix FileUtils for both just in case there's another or future way to get there in other GeckoView products (what snorp said in comment 11).

Does shipping Focus respond to or ignore those intents?

It does not support content:// at the moment.

Flags: needinfo?(snorp)

Comment on attachment 9130800 [details]
Bug 1617928 - Sanitize "content" uri filenames; r?AndreiLazar,snorp

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high - privacy escalation
  • User impact if declined: Network hijacking, settings overridden.
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small change
  • String or UUID changes made by this patch:
Attachment #9130800 - Flags: approval-mozilla-esr68?

Comment on attachment 9130800 [details]
Bug 1617928 - Sanitize "content" uri filenames; r?AndreiLazar,snorp

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Medium effort
  • 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?: All branches. Bug was probably introduced in bug 1499618
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: The solution is simple.
  • How likely is this patch to cause regressions; how much testing does it need?: Very low chances of regressions. Testing based on the provided poc should be enough.
Attachment #9130800 - Flags: sec-approval?
Summary: Firefox for Android: Directory Traversal can lead to network hijacking → Geckoview - Directory Traversal can lead to network hijacking
Summary: Geckoview - Directory Traversal can lead to network hijacking → Firefox for Android - Directory Traversal can lead to network hijacking

Comment on attachment 9130800 [details]
Bug 1617928 - Sanitize "content" uri filenames; r?AndreiLazar,snorp

Per discussion this is going to land under a public bug, with the comments changes so it looks less like a security fix.

Attachment #9130800 - Flags: sec-approval? → sec-approval+

As a public ticket I created bug 1622781.

Depends on: 1622781
Attachment #9130800 - Flags: approval-mozilla-esr68?
Group: mobile-core-security → core-security-release
Type: task → defect
Flags: sec-bounty? → sec-bounty+
Whiteboard: [reporter-external] [client-bounty-form] → [reporter-external] [client-bounty-form][adv-esr68.7+]
Alias: CVE-2020-6828
Group: core-security-release
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.