1638620 - Auto loading iframe with bad "intent://" scheme URL as source

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[__borelenzo__]

Attachment: intent_scheme.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Create a simple HTML page with the following iframe:
<iframe src="intent://#Intent;type=text/plain;action=android.intent.action.SEND;S.browser_fallback_url=http://evil.com;end" width="" height=""></iframe>

Actual results:

The value "scheme=http" is missing in the URI according to the documentation of Andoid Intent structure, leading to a crash: https://crash-stats.mozilla.org/report/index/e5650f51-a8b9-4d53-9d6f-570350200517

Expected results:

• Intent scheme should probably not be resolved as normal URLs in a browser, cause they can start unexpected activities or services
• the autoloading of the iframe makes it even more dangerous

A blank iframe should be displayed

Comment 1

[:Gijs (he/him)]

:snorp, can you take a look? Unclear if this affects Fenix, too.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

This looks like a fairly normal crash, as in it does not appear to be exploitable. While it is a DOS for Fennec we have more than a few ways to DOS Gecko that are public.

Comment 3

[__borelenzo__]

Indeed, the crash itself doesn't seem to be exploitable. I classified it as a potential security because it is automatically triggered when it is set as an iframe source (or programmatically clicked)

Comment 4

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

Doesn't crash Fenix here, but that code does still exist (and probably shouldn't).

Comment 5

[Petru-Mugurel Lingurar [:petru]]

@snorp Maybe Gecko / GV could filter schemes so not automatically load pages with executable src?
So to let something like this be executed only after user's action?
As a user I would've liked here an overlay with something like "This .. contains executable code. Touch to load and execute"

Regarding this crash in particular we crash because of the malformed intent which does not contain any info about the app that should be started

intent://#Intent;type=text/plain;action=android.intent.action.SEND;S.browser_fallback_url=http://evil.com;end

.. so we get that null pointer exception which is pretty easy to fix.

I think Fenix functions a little bit differently atm and it might be having another bug as it does not crash but nor does it load the fallback url.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1638620 - Improve safe intent handling for empty scheme uris; r?snorp

Prevent a NPE by first checking if the scheme for a given uri is null before
trying to normalize it.

Comment 7

[Daniel Veditz [:dveditz]]

Kevin: I believe we have another bug filed on not launching external intents without asking the user first? If not maybe this could be that bug. As with "mailto:" links on Desktop there might be a select few intents we could put on an allowlist for no interaction, but it should be a small list.

Comment 8

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Am I correct to assume that there is no data leak here, just a crash?

Comment 9

[Petru-Mugurel Lingurar [:petru]]

(In reply to Stefan Arentz [:st3fan] from comment #8)

Am I correct to assume that there is no data leak here, just a crash?

Atm we only have this relatively low volume crash that needs to be resolved.

Whether or not we should automatically launch external intents (like proposed by the reporter) I think warrants a bigger discussion and maybe snorp can offer more insights here.

Comment 10

[Kevin Brosnan [Ex-Mozilla]]

I believe we have that feature in Fenix. Bug 1182695 has some good info from mcomella about intent behaviors and gotchas. I would be against changing behavior in Fennec.

Comment 11

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

Yeah I think we do prompt when you're in a private tab at least? I agree, though, it's a bit late to be changing Fennec behavior.

Comment 12

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This patch looks good - how about we just land that for 68.10. I don't think this is very urgent considering the low crash volume.

Comment 13

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Petru, lets land this in the next release.

Comment 14

[Petru-Mugurel Lingurar [:petru]]

Try build to test the fix
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bc98495b46cf1bb580b68e546c4584510e16451

Comment 15

[Diana Rus]

Hi, verified the test build from previous comment, with Samsung Galaxy S9 (Android 8) and Google Pixel 3 XL (Android 9), good news, no more crash present.

As a remark on the test build the iframe is empty/blank and i also checked on Firefox Preview Beta 5.2.0 Gv:78 and no crash encountered but the domain from the intent(the fall back url) is accessed.

Comment 16

[Petru-Mugurel Lingurar [:petru]]

Thanks Diana.
Seems like there is a difference between Fenix and Fennec in how intent urls are handled.
And indeed when using intent urls from jsfiddle it seems like the fallback url is not being followed.
This behavior is not really tied to this ticket but might indicate a bigger problems. Will investigate.

Comment 17

[Petru-Mugurel Lingurar [:petru]]

Looked a bit at the above issue reported by Diana using the above apk from th (latest esr with the small patch to prevent the crash)
Saw that indeed accessing intent urls from jsfiddle like the one proposed here that would previously crash the app - https://jsfiddle.net/2w51tupv or a valid intent url - https://jsfiddle.net/zpncs598/show in an iframe would result in the fallback url not being loaded although we do extract the fallback url and then send it to gecko here.

Interestingly, loading the same code from a html file on disk will result in the fallback url being loaded in the iframe. Diana also confirmed.
So the difference between Fennec and Fenix would be that on Fennec the fallback url is loaded in the iframe while Fenix will open a new page for it.
Not sure what the problem is with regards to jsfiddle and if we should investigate more at this point.
Asking for snorp's opinion.

Otherwise, since this issue seems preexisting and we have a patch to resolved the crash I'll ask for uplift.

Comment 18

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9150111 [details]
Bug 1638620 - Improve safe intent handling for empty scheme uris; r?snorp

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Crash fix
• User impact if declined: Possible DOS - crash because of trying to (re)load a broken url.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small change, verified by QA.
• String or UUID changes made by this patch:

Comment 19

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

(In reply to Petru-Mugurel Lingurar [:petru] from comment #17)

Looked a bit at the above issue reported by Diana using the above apk from th (latest esr with the small patch to prevent the crash)
Saw that indeed accessing intent urls from jsfiddle like the one proposed here that would previously crash the app - https://jsfiddle.net/2w51tupv or a valid intent url - https://jsfiddle.net/zpncs598/show in an iframe would result in the fallback url not being loaded although we do extract the fallback url and then send it to gecko here.

Interestingly, loading the same code from a html file on disk will result in the fallback url being loaded in the iframe. Diana also confirmed.
So the difference between Fennec and Fenix would be that on Fennec the fallback url is loaded in the iframe while Fenix will open a new page for it.
Not sure what the problem is with regards to jsfiddle and if we should investigate more at this point.
Asking for snorp's opinion.

Otherwise, since this issue seems preexisting and we have a patch to resolved the crash I'll ask for uplift.

Yeah we probably need some changes to support intent fallbacks in iframes. Generally GV apps can't affect loads in non-toplevel frames.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9150111 [details]
Bug 1638620 - Improve safe intent handling for empty scheme uris; r?snorp

Fixes a Fennec crash. Approved for Fennec 68.10.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/bf0ecf78bee2

Comment 22

[Diana Rus]

Hi, verified this as fixed, as no crashes were present, on Firefox Release 68.10.0 with Sony Xperia Z5 (Android 7), Google Pixel 3XL (Android 9) and Samsung Galaxy S9 (Android 8).
As mentioned before, the fallback URL is not present when accessing from jsfiddle in the iframe and present/loaded in case of opening the html file with Fennec browser in the iframe.

Verified with:

• https://jsfiddle.net/zpncs598/show
• https://jsfiddle.net/2w51tupv
• and with an html file that contains the iframe