1149094 - (CVE-2015-2714) Mixed content violation log on Fennec leaks sensitive info in URL

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Muneaki Nishimura]

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2342.2 Safari/537.36

Steps to reproduce:

1. Install Fennec to an Android device which version is 4.0 or less.
2. Launch https://ie.microsoft.com/testdrive/browser/mixedcontent/assets/woodgrove.htm?SECRET#SECRET
3. Ignore SSL error and load the page
4. See logcat (via adb or some way)


Actual results:

Accessed URL contains query string and fragment is leaked to Logcat like below.

W/GeckoConsole( 1364): [JavaScript Warning: "Loading mixed (insecure) display content "http://ie.microsoft.com/testdrive/browser/mixedcontent/assets/images/small_biz.png" on a secure page" {file: "https://ie.microsoft.com/testdrive/browser/mixedcontent/assets/woodgrove.htm?SECRET#SECRET" line: 0}]

On Android 4.0 or less, any application having android.permission.READ_LOGS permission can retrieve other application's log data. And also, it may be sent to the third party as crash logs. So, sensitive information may be leaked.


Expected results:

Mixed content violation log should remove detailed information of URL,e.g, path, query string and fragment.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Mark, we already had a bug about not dumping console do logcat. Do you remember what happened there? I guess we never ended up turning that on?

Comment 2

[Daniel Veditz [:dveditz]]

If it's Android 4.0 or less doesn't that imply the android people realized log access wasn't such a smart idea, making it (at least partially) their problem?

Tanvi: could we simply strip query params from the log output here? It's not really relevant to the mixed content blocking message. then again the secret bit could be anywhere in the path -- the real problem is access to the logs, or dumping console output to the log.

Any web site can put stuff on the console log, that might be perfectly fine for debugging output (visible to the user or developer) and they aren't considering whether it's safe for general log access.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #2)
Yeah, in newer Android you can only view your own logs. Not other processes/activites/etc.

A malicious app would have to be installed as part of the system or by yourself. IMHO, you're already owned at that point, so us printing stuff to logcat isn't such a big deal.

Comment 5

[Tanvi Vyas[:tanvi]]

There are two urls here... the mixed content url and the url of the place where mixed content was found/injected.  Are we talking about removing querystring/hash for both of them?  Will this impact how we can click on the injection site and see where the mixed content was injected, along with the line number?

(Ex: open the webconsole, enable the security tab, and go to https://people.mozilla.org/~tvyas/mixedcontent.html on Firefox desktop).

Could we make the change android only since that is the only place where there is a security risk?

Aren't there other webconsole messages that leak similar data?  In that case, maybe the right solution here is to not dump console to logcat.

According to this, 4.0 or less is about 13% of the android market - https://developer.android.com/about/dashboards/index.html.  4.1 came out almost 3 years ago.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

We can do the following:
1. Ignore this. It's limited to older Android versions and you need to be powned by a malicious app to get into trouble.
2. We stop outputting nsIConsoleSevice output to the Android log for RELEASE_BUILD. So Nightly, Aurora and local developer builds would still have the output.
3. We add some Developer setting to the Settings, allowing developers to see the output.

The best I'm willing to do is #2, unless I get feedback that crash report logs without nsIConsoleSevice output hurts our ability to fix bugs.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: no-console-in-release v0.1

This patch drops the GeckoConsole output from RELEASE_BUILD (Beta and Final)

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8588594 [details] [diff] [review]
no-console-in-release v0.1

Approval Request Comment
[Feature/regressing bug #]: Leaking sensitive data to Android log
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low risk. Just stop dumping to Android log. Still shows up in the Browser Console.
[String/UUID change made/needed]: None

Comment 9

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

How far back does this issue go? A long way?

Comment 10

[Tanvi Vyas[:tanvi]]

Hi Mark,
Is there a way to do this for just Android 4.0 and less?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1cf4808e78d9

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8588594 [details] [diff] [review]
no-console-in-release v0.1

Should be in 38 beta 4

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/75799ecc00a7

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Tanvi Vyas[:tanvi]]

(In reply to Tanvi Vyas [:tanvi] from comment #10)
> Hi Mark,
> Is there a way to do this for just Android 4.0 and less?

needinfo'ing Mark for this question.  If there is, maybe we can file another bug to make this change.

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Tanvi Vyas [:tanvi] from comment #15)
> (In reply to Tanvi Vyas [:tanvi] from comment #10)
> > Hi Mark,
> > Is there a way to do this for just Android 4.0 and less?
> 
> needinfo'ing Mark for this question.  If there is, maybe we can file another
> bug to make this change.

Do we want to dump this to the Android Log at all? The output will still show up in the Firefox console and the Remote Debugging tools.

To answer your question, yes, we could probably add a runtime check, but it would be an additional, small performance issue.