1622278 - (CVE-2020-6827) Firefox for Android custom tabs URL spoofing

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, task)

Description

[Juho Nurminen [:jupenur]]

Attachment: spoof.html

Custom tabs in Firefox for Android fail to update the URL correctly, allowing malicious websites to spoof other origins.

Steps to reproduce:

1. Host the attached file (spoof.html) at the following URL: http://evil.example/spoof.html
2. Open the following URL in Firefox for Android: intent://evil.example/spoof.html#Intent;package=org.mozilla.firefox;action=android.intent.action.VIEW;scheme=http;S.android.support.customtabs.extra.SESSION=;end;
3. Observe how a custom tab opens, the URL changes to "https://google.com/", yet content is displayed from the "evil.example" host

Comment 1

[Johann Hofmann [:johannh]]

Snorp, Stefan, can you take a look, please?

Comment 2

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

I see the issue. We get some progress callbacks, including onPageStart() from GeckoView. The load is then aborted, and we get onPageStop() with an "unsuccessful" value passed along. Unfortunately, Fennec changes the displayed URL in onPageStart()[1] and never changes it back after onPageStop(). We should only be changing the displayed URL in onLocationChange().

Petru, can you please remove the line that I linked to below?

[1] https://searchfox.org/mozilla-esr68/source/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java#639

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Sure, thanks James!

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1622278 - Ensure displayed URL in modified only in onLocationChange(); snorp

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9133236 [details]
Bug 1622278 - Ensure displayed URL in modified only in onLocationChange(); snorp

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Security issue - url spoofing
• User impact if declined: Url spoofing in custom tabs. Users may be tricked about the real website origin.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small change
• String or UUID changes made by this patch:

Comment 6

[Daniel Veditz [:dveditz]]

Attachment: Screenshot_20200313-135914_Firefox Beta.jpg

Comment 7

[Daniel Veditz [:dveditz]]

Even if the URL is switched too early, why are we showing the lock if we haven't securely connected? (see screenshot) are we just parsing the scheme at that point?

Comment 8

[Ryan VanderMeulen [:RyanVM]]

This is Fennec-only code which only exists on ESR68.

Comment 9

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9133236 [details]
Bug 1622278 - Ensure displayed URL in modified only in onLocationChange(); snorp

Approved to land.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9133236 [details]
Bug 1622278 - Ensure displayed URL in modified only in onLocationChange(); snorp

Fixes a sec bug. Approved for Fennec 68.7b1.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Daniel Veditz [:dveditz]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)

This is Fennec-only code which only exists on ESR68.

I believe that is incorrect: Snorp said in comment 2 it regressed from when Fennec switched to using shared GeckoView code for this in bug 1356346. ni? him or petru to check me on that, and to see if this has landed on trunk and should be left fixed, or reopened until that happens.

Comment 13

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

(In reply to Daniel Veditz [:dveditz] from comment #12)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)

This is Fennec-only code which only exists on ESR68.

I believe that is incorrect: Snorp said in comment 2 it regressed from when Fennec switched to using shared GeckoView code for this in bug 1356346. ni? him or petru to check me on that, and to see if this has landed on trunk and should be left fixed, or reopened until that happens.

Right, the problem is in Fennec-only code which uses GeckoView, not in GV itself.

Comment 14

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt