Closed Bug 1638620 Opened 2 years ago Closed 2 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

Firefox 68
x86_64
Android
defect

Tracking

(firefox-esr6878+ verified)

RESOLVED FIXED
Tracking Status
firefox-esr68 78+ verified

People

(Reporter: borelenzo, Assigned: petru)

Details

(Keywords: csectype-dos, sec-other, Whiteboard: [adv-esr68.10-])

Crash Data

Attachments

(2 files)

Attached file 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

Severity: -- → S3
Points: --- → ?
OS: Unspecified → Android
Priority: -- → P3
Hardware: Unspecified → x86_64

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

Severity: S3 → --
Points: ? → ---
Component: Android partner distribution → General
Flags: needinfo?(snorp)
Priority: P3 → --
Assignee: mozilla → nobody

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.

Crash Signature: [@ java.lang.NullPointerException: at org.mozilla.gecko.util.IntentUtils.normalizeUriScheme(IntentUtils.java) ]
Flags: needinfo?(petru.lingurar)
Keywords: csectype-dos

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)

Status: UNCONFIRMED → NEW
Ever confirmed: true

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

Flags: needinfo?(snorp)

@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.

Flags: needinfo?(petru.lingurar)

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

Assignee: nobody → petru.lingurar
Status: NEW → ASSIGNED

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.

Flags: needinfo?(kbrosnan)

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

Flags: needinfo?(petru.lingurar)

(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.

Flags: needinfo?(petru.lingurar) → needinfo?(snorp)

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.

Flags: needinfo?(kbrosnan)

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.

Flags: needinfo?(snorp)

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.

Keywords: sec-other

Petru, lets land this in the next release.

Flags: needinfo?(petru.lingurar)

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.

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.

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.

Flags: needinfo?(petru.lingurar) → needinfo?(snorp)

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:
Attachment #9150111 - Flags: approval-mozilla-esr68?

(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.

Flags: needinfo?(snorp)

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.

Attachment #9150111 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Group: mobile-core-security → core-security-release

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:

Group: core-security-release
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.