Closed
Bug 1249092
Opened 8 years ago
Closed 8 years ago
GeckoProfile leaks profile path
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox46+ verified, firefox47 verified, fennec46+)
VERIFIED
FIXED
Firefox 47
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main46-])
Attachments
(1 file)
1.42 KB,
patch
|
rnewman
:
review+
lizzard
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
via https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java#268 Log.v(LOGTAG, "Fetching profile: '" + profileName + "', '" + profileDir + "'"); dveditz discovered it was added in FF42, bug 1145192 and mentioned, "the problem is that on older Android all apps can read that log, and if we store the profile on the SD card then many apps have permissions to read the profile". dveditz did some research: * We stopped moving the profile to the SD card in bug 721084 (FF14) * We fixed a bug to stop leaking profile paths in bug 953993 (FF27). Was this necessary? Richard, you fixed bug 953993 so maybe you have more context – do we still need to obscure the profile path? Some further facts/speculation on irc: 11:55 <dveditz> the problem is that on older Android all apps can read that log, and if we store the profile on the SD card then many apps have permissions to read the profile 11:55 <mcomella> I see 11:55 <mcomella> I was under the impression we didn't push the profile off to the SD card for that reason 11:55 <mcomella> But maybe that's changed? 11:56 <dveditz> or maybe it changed so that we don't push it to the SD card now, when we used to allow that as an option 11:56 <mcomella> I thought we kept the profile on internal memory but move the application itself to the sd card 11:56 <mcomella> In any case, I'll file a bug to investigate 11:57 <dveditz> even not on the SD card though, if an app knows the location they could send an intent to open a local file: in the browser 11:57 <mcomella> But if it's within the application's storage area, the permissions should be locked down so only the application can open it, right? 11:57 <dveditz> I heard we're dropping support for old android though. not sure what versions this is really an issue for 11:58 <mcomella> We are planning on dropping GB soonish, unclear when that is 11:58 <dveditz> you can make the browser download a file, in which case the browser owns it. 11:58 <dveditz> and the browser can open files in the browser profile, of course
Flags: needinfo?(rnewman)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → michael.l.comella
Comment 1•8 years ago
|
||
FYI, we are planning to drop support for GB in 48 (47 will be the last update).
Assignee | ||
Comment 2•8 years ago
|
||
Some more history: 12:08 <dveditz> it was actually added in 1077590, only moved in 1145192. The reviewed attachment differs from the commit :-( 12:08 <dveditz> looks like that bit was supposed to be removed -- https://bugzilla.mozilla.org/show_bug.cgi?id=1077590#c24
Comment 3•8 years ago
|
||
Obligatory history: Bug 56002 Bug 97180 Bug 944373 and pointers from those. Yes, Android 4.1 or 4.2 and higher prevents other apps from reading your logs (… apart from on rooted devices, and apart from system apps, that is). Notably, 4.1 is API 16, and so even if we kill GB, we'll still support API 15. That means we still have a justified policy of squashing this kind of logging.
Flags: needinfo?(rnewman)
Assignee | ||
Comment 4•8 years ago
|
||
I wonder if we can't write a test that will parse our logs and warn us if anything like a profile directory pops out...
tracking-fennec: --- → ?
Assignee | ||
Comment 5•8 years ago
|
||
MozReview-Commit-ID: BhK3YFbglee
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8725366 [details] [diff] [review] Log profile path in debug mode I'm unclear that we want to log this ever, but logging in debug mode seemed reasonable to me.
Attachment #8725366 -
Flags: review?(rnewman)
Updated•8 years ago
|
Attachment #8725366 -
Flags: review?(rnewman) → review+
Updated•8 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/ac846629e1df216445858e18057d0ef4e912fad2 Bug 1249092 - Log profile path in debug mode. r=rnewman
Comment 8•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ac846629e1df
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8725366 [details] [diff] [review] Log profile path in debug mode Approval Request Comment [Feature/regressing bug #]: bug 1145192 [User impact if declined]: Users profile path will be leaked into logcat output [Describe test coverage new/current, TreeHerder]: Tested locally [Risks and why]: Extremely low – we're adding a condition to our else branch and the variable is already existed/tested [String/UUID change made/needed]: None
Attachment #8725366 -
Flags: approval-mozilla-beta?
Attachment #8725366 -
Flags: approval-mozilla-aurora?
Comment 10•8 years ago
|
||
Comment on attachment 8725366 [details] [diff] [review] Log profile path in debug mode too late for 45. We release next tuesday
Attachment #8725366 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment 11•8 years ago
|
||
[Tracking Requested - why for this release]: low-risk security fix
tracking-fennec: ? → 46+
tracking-firefox46:
--- → ?
Comment 12•8 years ago
|
||
Regression since 42, I don't think we need to track.
status-firefox46:
--- → affected
Comment 13•8 years ago
|
||
Comment on attachment 8725366 [details] [diff] [review] Log profile path in debug mode Fix to avoid data leak, please uplift to aurora.
Attachment #8725366 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•8 years ago
|
Group: firefox-core-security → core-security-release
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage]
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main46-]
Comment 16•8 years ago
|
||
I was able to reproduce this on Firefox 45.0.2. Verified as fixed on HTC Desire S (Android 2.3.3) on Firefox 46 Beta 13 and on latest Nightly
Updated•7 years ago
|
Group: core-security-release
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•