Closed Bug 944374 (CVE-2014-1506) Opened 11 years ago Closed 11 years ago

Security vulnerability: CrashReporter Path Traversal

Categories

(Firefox for Android Graveyard :: General, defect)

25 Branch
x86_64
Android
defect
Not set
normal

Tracking

(firefox28 fixed, firefox29 fixed, firefox30 fixed, firefox-esr24 unaffected, b2g-v1.3 fixed)

RESOLVED FIXED
Firefox 30
Tracking Status
firefox28 --- fixed
firefox29 --- fixed
firefox30 --- fixed
firefox-esr24 --- unaffected
b2g-v1.3 --- fixed

People

(Reporter: roeeh, Assigned: bnicholson)

Details

(Keywords: csectype-disclosure, reporter-external, sec-moderate, Whiteboard: [reporter-external][adv-main28+])

Attachments

(2 files, 2 obsolete files)

Attached file 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.
The referenced weak randomness vulnerability in the whitepaper is Bug 944373.
OS: Windows 7 → Android
Flags: sec-bounty?
Whiteboard: [reporter-external]
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dveditz)
Ted: would this be yours to fix, or can you nominate someone on the Fennec team?
Assignee: nobody → ted
Flags: needinfo?(dveditz) → needinfo?(ted)
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.
Flags: needinfo?(ted)
Reassigning in an attempt to get some movement.
Assignee: ted → bnicholson
trivial updates
Attachment #8373801 - Attachment is obsolete: true
Attachment #8373801 - Flags: review?(blassey.bugs)
Attachment #8373809 - Flags: review?(blassey.bugs)
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 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())
Attachment #8373809 - Flags: review?(blassey.bugs) → review+
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?
Attachment #8374403 - Flags: review?(blassey.bugs)
Attachment #8374403 - Flags: review?(blassey.bugs) → review+
Target Milestone: --- → Firefox 30
Do we want/need to backport this at all?
Flags: sec-bounty? → sec-bounty+
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.
Attachment #8373809 - Attachment is obsolete: true
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
Attachment #8374403 - Flags: approval-mozilla-beta?
Attachment #8374403 - Flags: approval-mozilla-aurora?
Brian, what about ESR 24? if this vulnerability was always present, I think it should be backported too. Thanks
Flags: needinfo?(bnicholson)
Attachment #8374403 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
We don't ship an ESR version of Firefox for Android, AFAIK.
Right. Sorry, I didn't notice that it is an Android only issue.
Attachment #8374403 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [reporter-external] → [reporter-external][adv-main28+]
Alias: CVE-2014-1506
Group: core-security
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: