944374 - (CVE-2014-1506) Security vulnerability: CrashReporter Path Traversal

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Roee Hay]

Attachment: firefoxleakage.pdf

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.57 Safari/537.36

Steps to reproduce:

Technical details can be found in the attached whitepaper.


Actual results:

Technical details can be found in the attached whitepaper.


Expected results:

Technical details can be found in the attached whitepaper.

Comment 1

[Roee Hay]

The referenced weak randomness vulnerability in the whitepaper is Bug 944373.

Comment 2

[(not currently active) Ted Mielczarek]

So the gist here is that other apps installed on the device can invoke the CrashReporter intent with arbitrary file paths and get us to do stupid things. We should probably make a few changes here:
1) Reject minidump file paths that aren't in our application data directory. This should neuter most attacks here.
2) Possibly just hardcode the server URL into the CrashReporter app instead of reading it from the .extra file, so that even if a bad file gets in it can't force us to submit data to a third party.

These are more interesting on Android than desktop since on desktop anyone who could invoke the crashreporter app with an arbitrary path already has the ability to execute arbitrary code on your machine. On Android apps are expected to be sandboxed from each other so the threat is data leakage from our app.

Comment 3

[Daniel Veditz [:dveditz]]

Ted: would this be yours to fix, or can you nominate someone on the Fennec team?

Comment 4

[(not currently active) Ted Mielczarek]

I'm not real familiar with the Android crashreporter client, bnicholson has done work on it in the past. If he can't take this on blassey should be able to find someone.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Reassigning in an attempt to get some movement.

Comment 6

[Brian Nicholson (:bnicholson)]

Attachment: Abort if crash file is not descendant of application files dir

Comment 7

[Brian Nicholson (:bnicholson)]

Attachment: Abort if crash file is not descendant of application files dir

trivial updates

Comment 8

[Roee Hay]

I am afraid the suggested fix is not robust enough as the attacker can either
(1) Conduct a client-side DoS attack on Firefox by moving arbitrary files under the app dir to the pending path (See Section IV.B)
(2) Leak arbitrary files under the app dir by indirectly injecting the "ServerURL" directive into the file. (See Section IV.A.2.2).

Is there any reason why the CrashReporter activity is exported in the Android Manifest file?
If not, a simple fix would be to make it non-exported (and also make sure that there is no more complex data-flow which can taint the minidumpPath extra).

WDYT?

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8373809 [details] [diff] [review]
Abort if crash file is not descendant of application files dir

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

::: mobile/android/base/CrashReporter.java
@@ +119,5 @@
>  
>          String passedMinidumpPath = getIntent().getStringExtra(PASSED_MINI_DUMP_KEY);
>          File passedMinidumpFile = new File(passedMinidumpPath);
> +
> +        if (!isDescendantOf(passedMinidumpFile, getFilesDir())) {

The one thing I want to confirm before this lands is crash reports for non-default profiles (like our testing profiles on sd card) pass this test

@@ +471,5 @@
>      private String unescape(String string) {
>          return string.replaceAll("\\\\\\\\", "\\").replaceAll("\\\\n", "\n").replaceAll("\\\\t", "\t");
>      }
> +
> +    private boolean isDescendantOf(File file, File dir) {

seems like you could also do file.getCannonicalPath().startsWith(dir.getCannonicalPath())

Comment 10

[Brian Nicholson (:bnicholson)]

Attachment: Alt fix: Don't export CrashReporter activity

Here's an alternate fix that doesn't export CrashReporter as suggested in comment 8. This should prevent attackers from passing in other files owned by us (e.g., files in the cache dir). Things I've tested with this patch:

* Triggering the crash reporter on 4.2+ device (resulting in androidUserSerial code path here: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#738) still works
* Triggering crash reporter on < 4.2 device (non-androidUserSerial code path above) still works
* Launching the intent from another activity, which fails as expected with a "not exported" SecurityException

Anything else worth testing?

Comment 11

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/fx-team/rev/dfde6f7df7f5

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/dfde6f7df7f5

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Do we want/need to backport this at all?

Comment 15

[Daniel Veditz [:dveditz]]

Awarding a bounty for this sec-moderate bug because malicious Android apps abound. It may not be as widespread as a web-page drive-by attack but you could convince lots of people to install and try any half-interesting-looking game type thing.

Comment 16

[Brian Nicholson (:bnicholson)]

Comment on attachment 8374403 [details] [diff] [review]
Alt fix: Don't export CrashReporter activity

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This has always been a vulnerability of CrashReporter.
User impact if declined: Malicious apps could manipulate the crash reporter
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low risk; prevents activities outside of Fennec from invoking the crash reporter.
String or IDL/UUID changes made by this patch: None

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Brian, what about ESR 24? if this vulnerability was always present, I think it should be backported too.
Thanks

Comment 18

[(not currently active) Ted Mielczarek]

We don't ship an ESR version of Firefox for Android, AFAIK.

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Right. Sorry, I didn't notice that it is an Android only issue.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4974f42186b9

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dfc09d89b475

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/dfc09d89b475