1521708 - Show about:blank (placeholder "Search or enter address" in the URL bar) and bypass insecure login warning

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Abdulrahman Alqabandi]

Attachment: spafer.html

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

Steps to reproduce:

open("","q","q").document.body.innerHTML=123;

or see attached for an attack scenario, simply open the html and click on link.

Actual results:

blank (placeholder) address bar

Expected results:

per bug 1284395 and bug 1228754 any window that has an opener should always display the full 'about:blank' to prevent confusing users.

As a side effect it seems like this also bypasses insecure login warnings, if we open 'about:blank' in a new tab from an insecure website (not popup window as working poc) first thing we will notice is that 'about:blank' appears normally in the bar, and if we inject a login form then as soon as we reach the password input field we will get an insecure warning. However, if we use the PoC attached you will see that no warning appears and the address bar is blank (with placeholder).

Tested only on latest nightly.

Comment 1

[Abdulrahman Alqabandi]

Attachment: Resulting screenshot

screenshot of resulting popup.

Comment 2

[Johann Hofmann [:johannh]]

The about:blank part is definitely a regression of bug 1362774. I didn't check for the insecure login warning bypass and I would recommend filing a separate bug for it.

Gijs, can you take a look?

Comment 3

[:Gijs (he/him)]

Copied from some IRC discussion:

15:28:13 <Gijs> basically, we stopped hitting this block:
15:28:14 <Gijs> https://searchfox.org/mozilla-central/rev/330daedbeac2bba296d663668e0e0cf248bc6823/browser/base/content/tabbrowser.js#5192-5195

if (!this.mBlank) {
  this._callProgressListeners("onLocationChange",
                              [aWebProgress, aRequest, aLocation, aFlags]);
}

(in onLocationChange in tabbrowser)

15:28:28 <Gijs> I think the Real fix is to make us not initialize mBlank to true anymore
15:28:34 <Gijs> but there are 2 issues with this
15:28:50 <Gijs> (1) we do still often create an about:blank doc ( see e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1471327 )
15:29:02 <Gijs> (2) there'd probably be fallout from such a change
15:29:11 <Gijs> a cheap fix is to make that block check:
15:29:21 <Gijs> if (!this.mBlank || this.mBrowser.hasContentOpener)
15:29:39 <Gijs> and thus force us to dispatch the onLocationChange notification in the window.open case
15:29:49 <Gijs> that's sufficient to fix this bug and, I hope/expect, has minimal risks

Not setting mBlank would involve changing https://searchfox.org/mozilla-central/rev/330daedbeac2bba296d663668e0e0cf248bc6823/browser/base/content/tabbrowser.js#378 , which will likely have significant fall-out. I'm happy to file a follow-up for that but I don't think we should do it here.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1521708, r?johannh

Comment 5

[:Gijs (he/him)]

Gonna go ahead and call this sec-moderate given that's what the other bugs referenced in comment #0 (with similar spoofing outcomes) were. If the sec group wants to alter that please do.

Comment 6

[:Gijs (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2775f83185e

Comment 7

[:Gijs (he/him)]

ni for me to doublecheck about getting a beta/release patch here because the thing I just landed on inbound will conflict because of bug 1514164.

Comment 8

[:Gijs (he/him)]

Comment on attachment 9038242 [details]
Bug 1521708, r?johannh

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1362774

User impact if declined: location bar spoof risk

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: See comment #0 (note: only URL bar change is fixed here, not the password warning)

List of other uplifts needed: n/a

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): one-line change that ensures we update the location bar when the location change event fires for new windows being opened

String changes made/needed: n/a

Comment 9

[:Gijs (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1c7d5975a884c86a3c31bdd19049878494d5572

Comment 10

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

(In reply to :Gijs (he/him) from comment #5)

Gonna go ahead and call this sec-moderate given that's what the other bugs
referenced in comment #0 (with similar spoofing outcomes) were. If the sec
group wants to alter that please do.

This seems reasonable to me.

Comment 11

[Abdulrahman Alqabandi]

Well done on the swift response. I believe the insecure login problem will go away once this is fixed, it seems that so long as "about:blank" is shown int he address bar it will work fine. Will make sure once patch lands.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e2775f83185e
https://hg.mozilla.org/mozilla-central/rev/d1c7d5975a88

Comment 13

[:Gijs (he/him)]

Attachment: Patch for mozilla-release

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9038476 [details] [diff] [review]
Patch for mozilla-release

[Triage Comment]
Fixes a new location bar spoofing issue introduced in Fx65. Approved for 65.0 RC2.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/7dfc526e16ef

Comment 16

[Stefan [:StefanG_QA]]

I was able to reproduce the issue on Nightly build from 01-16-2019. (20190116170105)

The issue is verified as fixed on the below builds using the spafer.html file from comment 0. Now 'about:blank' is displayed in the URL address bar.

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0 (20190128092811)

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0 (20190124174741)

Comment 17

[:Gijs (he/him)]

(In reply to Stefan [:StefanG_QA] from comment #16)

I was able to reproduce the issue on Nightly build from 01-16-2019. (20190116170105)

The issue is verified as fixed on the below builds using the spafer.html file from comment 0. Now 'about:blank' is displayed in the URL address bar.

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0 (20190128092811)

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0 (20190124174741)

Thanks, setting the per-version flags as well. :-)

Comment 18

[Cornel Ionce [:noni] [Hubs QA]]

Removing the qe-verify+ flag since it was already verified.