1208525 - (CVE-2016-1940) Address bar spoofing with a BOOKMARK shortcut of data: URL

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Muneaki Nishimura]

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

Steps to reproduce:

1. Load a data: URL from a bookmark shortcut on a home screen
2. The data: URL moves to an other http(s): URL

You can reproduce it by following steps.
1. Close Fennec app at first as a preparation
2. Execute "adb shell" on a console PC that is connected to the device via USB
3. Execute following command that invokes an explicit BOOKMARK intent to the app.
am start -a org.mozilla.gecko.BOOKMARK -d
data:text/html\;base64,UHduPHNjcmlwdD5zZXRUaW1lb3V0KCd3aW5kb3cubG9jYXRpb249YGh0dHBzOi8vd3d3Lmdvb2dsZS5jb21gLDEwMDAnKTwvc2NyaXB0Pg==
-n org.mozilla.fennec/org.mozilla.gecko.BrowserApp
3. A few seconds later, the intent: page is moved to https://www.google.com


Actual results:

Address bar of Fennec doesn't change from data: URL to https://www.google.com  that is shown on Fennec.


Expected results:

https://www.google.com should be shown on an address bar correctly.

Comment 1

[Muneaki Nishimura]

Attachment: You can see it if reproduced

Comment 2

[:Margaret Leibovic]

Ally, interested in looking into this one?

Comment 3

[Allison Naaktgeboren :ally]

I've signed up for the other one, but I can take a crack at this too. I just hope I'm not overcommitting!

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: decoded base64 content

I threw the base 64 url in desktop to see what happens and the browser prompts to "download" the file. I opened the file with Firefox which notably did not execute as HTML.

Perhaps the difference here is that the url is going through a home screen shortcut?

Still, it's strange the title does not update given it's a `window.location=...` call. I wonder if this is on Gecko. Perhaps a side effect of not being started yet?

Comment 5

[Allison Naaktgeboren :ally]

Comment on attachment 8666066 [details]
You can see it if reproduced

Could we get a sec rating on this bug?

I'll be leaving for a couple weeks of pto and we need to know if someone else needs to pick this up, or if this can wait until I get back sometime in November.

Thanks!

Comment 6

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

Comment on attachment 8666066 [details]
You can see it if reproduced

sec-approval? is for patches, not security requests in general. Clearing it.

I'll see if we can get a rating here.

Comment 7

[Allison Naaktgeboren :ally]

moving over from email discussion:

I will be out until November. The outstanding question is whether or not this is kosher to wait until I return, or should be fixed by someone in the interim. The answer seems to hinge on the probability & extent of attacks a normal, non tech savvy user would be exposed to.

Below is my best understanding.

It looks like its any app that can issue bookmark intents ("3. Execute following command that invokes an explicit BOOKMARK intent to the app."). Users don't have control over malicious apps sending intents to other apps. In general attacks from other apps concern me as every smart phone user has other apps. Free games in particular, I understand, are a popular route to attack other apps on the phone. 

URLs from the web look to be spoofable as well based on Muneaki's original filing("1. Load a data: URL from a bookmark shortcut on a home screen") because if a user saves a link as a bookmark and then long taps on it to "Add it to homescreen" which adds it to his/her android homescreen which when clicked loads spoofing link in fennec.

There's a bit of a language barrier, but that's what I think he means in his first comment by the first set of instructions, and the second covers other apps sending intents.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #7)
> URLs from the web look to be spoofable as well based on Muneaki's original
> filing("1. Load a data: URL from a bookmark shortcut on a home screen")
> because if a user saves a link as a bookmark and then long taps on it to
> "Add it to homescreen" which adds it to his/her android homescreen which
> when clicked loads spoofing link in fennec.

To demonstrate a potential attack vector, the user is on a website that says, "Long press to add Google to your homescreen!" and the user does this. The user clicks the homescreen shortcut, which is a website spoofing Google, and can only verify this is not actually Google by the url, which I don't trust all users do. The website can then take actions such as stealing the user's credentials.

This sounds higher than sec-moderate to me, but I'm not familiar with the security ratings.

Comment 9

[:Margaret Leibovic]

ahunt, can you look into this?

Comment 10

[Andrzej Hunt :ahunt]

Attachment: 1208525_locationchange.patch

It turns out we were throwing an Exception when trying to retrieve the host (which doesn't exist for data: URIs) for the previously shown URI while loading a new URI - this prevented the Content:LocationChange/LOCATION_CHANGE events from being fired.

The attached a patch which fixes this - I couldn't find a way to determine whether the URI should have a host, which is why my patch just catches the exception when we access the host instead.

Comment 11

[:Margaret Leibovic]

Comment on attachment 8703847 [details] [diff] [review]
1208525_locationchange.patch

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

::: mobile/android/chrome/content/browser.js
@@ +4531,5 @@
> +        let originHost = Services.io.newURI(appOrigin, null, null).host;
> +      } catch (e if (e.result == Cr.NS_ERROR_FAILURE)) {
> +        // NS_ERROR_FAILURE can be thrown by nsIURI.host if the URI scheme does not possess a host - in this case
> +        // we just act as if we have an empty host.
> +      }

With this fix, this means that we'll always open new tabs for homescreen shortcuts that don't have valid hosts. Maybe we should check to see if the URI is exactly the same before falling back to comparing hosts.

However, we could do that in a follow-up patch, since this seems like a low-risk way to avoid problems with these edge-case URIs.

Comment 12

[Andrzej Hunt :ahunt]

https://hg.mozilla.org/integration/fx-team/rev/5a6b63cd300eb05f28b54e4f5266406a29e13b7a
Bug 1208525 - don't fail to send LocationChange if the previous URI has no host r=margaret

Comment 13

[Andrzej Hunt :ahunt]

Comment on attachment 8703847 [details] [diff] [review]
1208525_locationchange.patch

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: Website URL can be spoofed by a data:/ URL (whereas patch ensures the correct URL is always shown).
[Describe test coverage new/current, TreeHerder]: Patch has been tested manually, no further verification yet.
[Risks and why]: Low risk - we are now catching an Exception that was previously ignored.
[String/UUID change made/needed]: n/a

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5a6b63cd300e

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8703847 [details] [diff] [review]
1208525_locationchange.patch

sec-fix, taking it.

Comment 16

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/aadb131b7184
https://hg.mozilla.org/releases/mozilla-beta/rev/97c3b29fb4f6

Comment 17

[Matt Wobensmith [:mwobensmith][:matt:]]

Hello Muneaki, are you able to confirm that this has been fixed in the latest Firefox?

Comment 18

[Muneaki Nishimura]

It seems to be fixed since BOOKMARK intent receiver was removed.