1679556 - Investigate removing alternate fixup to avoid Firefox redirecting http://https://.. to http://https.com/ (or other malware sites)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Theodric Young]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0

Steps to reproduce:

Type the following into the address bar: http://https://mozilla.org/

Actual results:

Firefox redirected to http://https.com/ , which then redirects to a number of malware sites.

Expected results:

Firefox should have indicated a syntax error in the URL (or perhaps taken some other action). I understand the motivation to "guess" what website the user wanted to visit by appending dot-com to the end of the server name. However, it appears that https.com is currently owned by a bad actor and certainly is NOT what a user is expecting to visit if, for whatever reason, the string, "http://" is appended to the front of a legitimate URL.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I think we should probably just detect the mistype and go to https://domain (in case multiple http/https are typed, just assume https).
Fwiw, what we do is not wrong, "https:" can be a valid local domain with an empty port, when we fail to visit it, we try to be "smart" and add ".com" to it.
I'm surprised that https.com is not in Safe Browsing lists considering what it is serving.
Gijs, what do you think, should we consider this part of protocol fixup in URIFixup?

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

another alternative could be to never try to add an alternate suffix to certain words like http, https, www... this may be better since it won't disallow one to actually have a local server named http or https.

Comment 4

[:Gijs (he/him)]

I'm surprised this wasn't fixed in bug 1181081 - but yes, I think we should fix it in URIFixup.

(In reply to Marco Bonardo [:mak] (Back Nov. 30th) from comment #3)

another alternative could be to never try to add an alternate suffix to certain words like http, https, www... this may be better since it won't disallow one to actually have a local server named http or https.

We're definitely in diminishing returns territory already. Looks like other browsers just don't do alternate fixup, so I guess that works. We could also avoid correcting it for http://https/foo, but correct when the colon and/or double / are present.

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

The idea is that we fix multiple http/https protocols, if even just one of them is https, correct to https.
Double slashes should be always present imo, because http://https:/foo looks a bit more valid than http://https://foo, or http://https//foo, we don't want to prevent having a local server named http or https, even if it looks a bit strange.
In addition we can avoid generating an alternate fixup for a set of words including: http, https, www.

I'd also be ok with evaluating the removal of the alternate fixup suffix, I understand trying to add www., but I'm not sure why we try to transform a local host into a .com domain, it sounds like at a maximum the network error page should say something like "Did you mean to go to [foo.com]?", rather than being automatic.

Comment 6

[:Gijs (he/him)]

(In reply to Marco Bonardo [:mak] from comment #5)

I'd also be ok with evaluating the removal of the alternate fixup suffix, I understand trying to add www., but I'm not sure why we try to transform a local host into a .com domain, it sounds like at a maximum the network error page should say something like "Did you mean to go to [foo.com]?", rather than being automatic.

I think we could remove it when the original input is not prefixed with a protocol. With the new architecture where we don't resolve in docshell, this may be possible? Though, are we hitting this path from the docshell fixup or from the address bar fixup?

The main issue with removing it altogether, AIUI, is the group of users who disable search in the address bar (keyword.enabled).

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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

I think we could remove it when the original input is not prefixed with a protocol. With the new architecture where we don't resolve in docshell, this may be possible? Though, are we hitting this path from the docshell fixup or from the address bar fixup?

It depends. If there's no protocol and the domain is not known (in domainwhitelist pref) the urlbar will just search. If the domain is known, we likely don't want to use alternate.
So in most cases this only affects:

1. urls typed with a protocol. If one types the protocol either they understand urls and alternate is not particularly useful, or it's a copy/paste from a bogus source
2. urls without a protocol but with a known domain. If we use alternate we do the wrong thing.
3. broken links on pages, and here we open us to possible hijacks like comment 0

It is the docshell that uses alternate fixup, the urlbar doesn't care about it.

The main issue with removing it altogether, AIUI, is the group of users who disable search in the address bar (keyword.enabled).

Could you elaborate please? I expect that group to be the one that will suffer less from a removal, since they'll be more than able to understand they typed a wrong url and act on it. It's also (didn't check though) a micro-tiny group, that we support, but we should not design after it.
Do you expect people in that group to be typing domains without a suffix, like typing amazon and expect to end on amazon.com? Wouldn't autofill help them anyway?

Comment 8

[:Gijs (he/him)]

(In reply to Marco Bonardo [:mak] from comment #7)

2. broken links on pages, and here we open us to possible hijacks like comment 0

I would not expect us to do any correction for broken links, alternate or otherwise...

It is the docshell that uses alternate fixup, the urlbar doesn't care about it.

Ah. :-(

The main issue with removing it altogether, AIUI, is the group of users who disable search in the address bar (keyword.enabled).

Could you elaborate please? I expect that group to be the one that will suffer less from a removal, since they'll be more than able to understand they typed a wrong url and act on it. It's also (didn't check though) a micro-tiny group, that we support, but we should not design after it.

Fair.

Do you expect people in that group to be typing domains without a suffix, like typing amazon and expect to end on amazon.com?

Yes.

Wouldn't autofill help them anyway?

Maybe, if they have history/bookmarks that provide that info. Anyway, we could easily turn it off by default and see what negative feedback we get, if any?

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Yes, we could disable suffix fixup behind a pref, but we should also measure that pref through telemetry to check how many will flip it, we're driving blindly otherwise.

Comment 12

[Steven Penny]

In my opinion, a browser should never, ever, add text into the middle of a URL, period.

I cant think one case where this would do more good than harm. If I am wrong, please provide an example as I am willing to change my mind.

If a browser want to add at the FRONT, like this:

example.com -> http://example.com

or add to the BACK, like this:

http://example -> http://example.com

then fine. But adding text into the MIDDLE of a URL is not acceptable, and shouldnt be Mozilla works very hard to protect users, with now siloed cookie protection, and many number of other protections. Why allow for such a silly attack vector, one that you created yourself?

Comment 13

[:Gijs (he/him)]

(In reply to Steven Penny from comment #12)

In my opinion, a browser should never, ever, add text into the middle of a URL, period.

It's adding www. and .com to the start and end of the hostname. The code that does this is not (and cannot be) aware of whether you typed the http: in the URL bar to start with or not. It only runs after the load of http://example/ has already happened (and failed, because the example host doesn't exist), at which point either the http:// was already present or was added by the URL bar to make a valid URL - but the code doing the fix up has no way of knowing. URL parsing means the second :/ in the http://https:// example is interpreted as an empty port and dropped, and also not available to the code in question.

There is no need to further argue that this is a bad experience - we are aware and agree with you. But fixing it either requires turning this feature off entirely, which impacts other workflows, too (entering just "example" in the address bar with search turned off opening "example.com"), or, if we want to preserve other workflows, is technically difficult because of the different layers at which the parsing and decision making steps happen (before / after DNS lookups). In other words, we cannot easily "just" avoid doing this "in the middle of a URL".

Comment 14

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

The solutions I suggest here involve:

1. disable suffix fixup (with a legacy pref initially), add that pref to the telemetry env so we can measure if it's being flipped back and by how many. If the numbers are uninteresting just remove the pref.
2. improve the notfound page to suggest a suffix fixed url, if it's possible to build one. Though, maybe we should first run a dns query and check it exists, suggesting a nonexisting page on a notfound page, is not particularly useful.

Comment 15

[Mathew Hodson]

I think this was fixed by bug 1719442

Comment 16

[:Gijs (he/him)]

(In reply to Mathew Hodson from comment #15)

I think this was fixed by bug 1719442

The worst behaviour is gone, but ideally we would still do something better than showing a network error page (like correcting the typo), and there is also discussion in this bug around disabling the "alternate" (www/com prefix/suffix) fixup altogether, so I'm not convinced this should be closed, though perhaps if we keep it the summary should be updated. Marco, WDYT?

Comment 17

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Yeah, I think we should really investigate the possibility to remove alternate fixup, but in my opinion to do that properly the NotFound page should be improved, by at least providing the alternate url on it, sort-of a "did you instead mean to go to https://www.typedword.com/"?

As I wrote in comment 7, I currently don't see a strong value in the alternate fixup feature with the urlbar properly redirecting single words to search. Only a niche of technical users disabling such behavior, users that can very well understand the difference between "someword" and "someword.com" and that even more likely don't want us to manipulate their string and visit unexpected target sites.
And I don't think we should care trying to fix broken links on pages, considering that doesn't come with zero risks.

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:adw, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 20

[James Teow [:jteow]]

Attachment: Bug 1679556 - Toggle browser.fixup.alternate.enabled and add telemetry, r?mak

The suggested solution for this bug is to disable browser.fixup.alternate.enabled
unless toggled by the user, and to add telemetry to the preference so that if we
find users rarely use the option, we can safely remove the feature in a future release.

Many tests rely on expecting URI's to be modified, so I modified them so that they
should not expect the prefix or suffix to be modified.

Comment 21

[James Teow [:jteow]]

Attachment: data-request.md

Hi there :chutten, I'm requesting a data review for a patch that's still in progress though what we are hoping to add to telemetry shouldn't change, at least based on the engineering proposal.

Comment 22

[Travis Long [:travis_]]

Comment on attachment 9280545 [details]
data-request.md

Data Review

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, documentation will ultimately live at https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html

2. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, through the standard telemetry preference

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

Permanent collection to be monitored by James Teow <[e-mail]>

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2

5. Is the data collection request for default-on or default-off?

Default-on

6. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No

7. Is the data collection covered by the existing Firefox privacy notice?

Yes

8. Does the data collection use a third-party collection tool?

No

Result

data-review+

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7bd4cbb9bdd0
Toggle browser.fixup.alternate.enabled and add telemetry, r=mak

Comment 24

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/7bd4cbb9bdd0

Comment 25

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue with Firefox 85.0a1 (20201127155321) on Windows 10x64. Loading http://https://mozilla.org/ link will redirect to malware webpages.
The issue is verified fixed with Firefox 104.0b2 (20220726185717) on Windows 10x64, macOS 11 and Ubuntu 20.04. Server Not Found page is displayed when loading the http://https://mozilla.org/ link.