1724473 - www is prepended to invalid urls like '.de' and '.com' and thereby can redirect to shady websites

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Lukas Jung]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

type just a dot and a tld (e.g. '.de' or '.com') in the address bar and hit enter.

Actual results:

for '.de':
firefox resolves 'www.de', which redirects to some apparently random website.

Expected results:

Firefox should show Google (or the default search engine) searching for '.de'.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Search' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Drew Willcoxon :adw]

Thanks for reporting this. What happens is Firefox treats .de as a valid domain name and tries to load it. That fails, so then Firefox falls back (by default) to prepending www and trying to load that.

Gijs, I wasn't sure whether leading dots are valid in domain or hostnames, but based on some quick research it doesn't appear so. Fixup treats .de as a valid domain -- it returns a fixed URI of http://.de/ -- and new URL() and Services.io.newURI() do too. Shouldn't fixup fail or at least remove the dot if FIXUP_FLAG_FIX_SCHEME_TYPOS is given?

As it is, with fixup (and new URL()) allowing .de as a valid domain, the address bar is doing the "right" thing. If we don't modify fixup to disallow it, then this is probably wontfix.

Comment 3

[:Gijs (he/him)]

(In reply to Drew Willcoxon :adw from comment #2)

Thanks for reporting this. What happens is Firefox treats .de as a valid domain name and tries to load it. That fails, so then Firefox falls back (by default) to prepending www and trying to load that.

Gijs, I wasn't sure whether leading dots are valid in domain or hostnames, but based on some quick research it doesn't appear so.

FWIW, in future please state what you found where, now I just get to do the same research...

As best I can tell, the URL spec suggests that a valid host string should mean that:

A domain is a valid domain if these steps return success:

Let result be the result of running domain to ASCII with domain and true.

which boils down to running the IDNA spec ToAscii with VerifyDnsLength set to true, which AFAICT says that splitting the domain names on . should leave only labels of length 1 or more (with some max lengths that are not relevant to the issue at hand), which is odd because trailing dots are definitely used in the URL/nslookup world (ie https://example.com./ works and is different from https://example.com/, which the URL spec explicitly highlights), and would leave an empty (0-length) label at the end. I don't see the IDNA spec clarifying anything specifically about the empty labels, though it re-joins the labels afterwards. So I'm confused... Anne, can you help?

Fixup treats .de as a valid domain -- it returns a fixed URI of http://.de/ -- and new URL() and Services.io.newURI() do too.

The order of these statements is interesting. :-)

The first thing fixup does is just passing the URL to the URL parser, if necessary prefixed by http://. It doesn't do a whole lot of fixup if the result is valid, except for typos in the scheme, and for hosts that have no . characters at all and didn't have scheme input (which we assume were intended as searches, rather than attempts to load http://foo/ which won't work for most people).

So if the URL parser doesn't complain, the scheme is correct, and there is at least one ., fixup won't do much itself. Once we hit the docshell with the original URI, it'll not find a host to connect to, which triggers the "alternate URI" fallback which is how we end up on www.de.

Shouldn't fixup fail or at least remove the dot if FIXUP_FLAG_FIX_SCHEME_TYPOS is given?

Well, this isn't a scheme typo as such (like htp://example.com would be), so I don't think so. Perhaps it should do a search if the relevant fixup flag was passed (which the URL bar does do in most cases), but I'd definitely like to have a more well-founded idea of which domains are and are not guaranteed to exist (e.g. should .org.uk also search? What about .example.com ?) on both the public internet and on intranets... and right now I'm not sure. Perhaps .<tld> should do a search but .host.<tld> should just silently drop the initial . ? Again, I don't know off-hand what is and isn't valid (not just usually/theoretically, but ever).

Comment 4

[Lukas Jung]

The behavior isn't as consistent as I thought:
.de goes to www.de
.com goes to www.com
.org goes to www.org and then to w3.org

BUT

.localhost
.co.uk
.nl
.fr
.sncf (tld of the french railway company)

go directly to Google

So this bug is not just about whether an url is valid.

Comment 5

[:Gijs (he/him)]

(In reply to Lukas Jung from comment #4)

The behavior isn't as consistent as I thought:
.de goes to www.de
.com goes to www.com
.org goes to www.org and then to w3.org

BUT

.localhost
.co.uk
.nl
.fr
.sncf (tld of the french railway company)

go directly to Google

So this bug is not just about whether an url is valid.

This suggests there's something PSL-based that is happening, but I'm not sure off-hand what that would be...

Comment 6

[Marco Bonardo [:mak]]

This is apparently a bug in URIFixup, because
Services.eTLD.getKnownPublicSuffix(makeURI("http://a.de/")) => .de
Services.eTLD.getKnownPublicSuffix(makeURI("http://.de/")) => throws
But the bailout here considers .de valid:
https://searchfox.org/mozilla-central/rev/7bafa9b9c2156018ec1d410194f9bf4b5b23e77f/docshell/base/URIFixup.jsm#668-675

I don't think we need to check specifics or such, we should just fix the bailout to require at least one alfanumeric before the suffix.

Comment 7

[Anne (:annevk)]

Gijs, I think you found a bug in UTS46 (it not accounting for trailing dots). I've reported it (unfortunately no reference as their issue tracking is somewhat of a black hole). As far as the URL Standard is concerned .whatever is invalid, but it will parse as host (and then fail on the network layer). It seems reasonable for the address bar to special case it therefore.

(Arguably anything is reasonable as it's very much a question of what we deem to be appropriate UI, but certainly in the case where it would result in failure we could do something else.)

Comment 8

[Marco Bonardo [:mak]]

The fix suggested in comment 6 can be mentored, it will need a test, there's other URIFixup tests to use for inspiration under https://searchfox.org/mozilla-central/source/docshell/test/unit

Comment 9

[Tanju Brunostar]

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

The fix suggested in comment 6 can be mentored, it will need a test, there's other URIFixup tests to use for inspiration under https://searchfox.org/mozilla-central/source/docshell/test/unit

I'd like to contribute on this. If someone is not working on it already

Comment 10

[Marco Bonardo [:mak]]

Feel free to start working on it, I didn't see others asking so far.
Note that mentored bugs are automatically assigned at the time the first patch is attached to them.

Comment 11

[Tanju Brunostar]

how do I resolve this bug. I've tried all I can to no avail. Please help

Comment 12

[Marco Bonardo [:mak]]

What did you try?
Comment 6 is the starting point here, there is a code bailout (see the link there) that just checks if the host ends in .something, that bailout should only apply if the strings starts with an alphanumeric, that's still an approximation but it should do for our needs. You can just use a Regex for that.
Then you should be able to add a test to docshell/test/unit/test_URIFixup_info.js checking that .de runs a query instead of browsing to http://.de/ (check keywordLookup: true).

Comment 13

[Tanju Brunostar]

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

What did you try?
Comment 6 is the starting point here, there is a code bailout (see the link there) that just checks if the host ends in .something, that bailout should only apply if the strings starts with an alphanumeric, that's still an approximation but it should do for our needs. You can just use a Regex for that.
Then you should be able to add a test to docshell/test/unit/test_URIFixup_info.js checking that .de runs a query instead of browsing to http://.de/ (check keywordLookup: true).

// Quick bailouts for most common cases, according to Alexa Top 1 million.
if (
asciiHost.endsWith(".com") ||
asciiHost.endsWith(".net") ||
asciiHost.endsWith(".org") ||
asciiHost.endsWith(".ru") ||
asciiHost.endsWith(".de")
) {
if (/^\w/.test(asciiHost)){
return {
suffix: asciiHost.substring(asciiHost.lastIndexOf(".") + 1),
hasUnknownSuffix: false,
};
}
}
exactly what I did here but once I save, rebuild and run firefox, it doesn't seem to solve the problem

Comment 14

[Marco Bonardo [:mak]]

It seems to be working for me, these are my STR:

1. type ".de" in the urlbar, confirm with enter
2. Check a search for ".de" is executed, instead of visiting http://.de

Please check you rebuilt correctly and that you don't have some preferences set, eventually you can use ./mach run --temp-profile to run in a clean profile.

Comment 15

[Tanju Brunostar]

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

It seems to be working for me, these are my STR:

1. type ".de" in the urlbar, confirm with enter
2. Check a search for ".de" is executed, instead of visiting http://.de

Please check you rebuilt correctly and that you don't have some preferences set, eventually you can use ./mach run --temp-profile to run in a clean profile.

maybe I'm not rebuilding correctly. I used "./mach build"
I actually commented out everything in the URIFixup.jsm module and rebuilt but nothing changed when I ran. no effect at all

Comment 16

[Tanju Brunostar]

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

It seems to be working for me, these are my STR:

1. type ".de" in the urlbar, confirm with enter
2. Check a search for ".de" is executed, instead of visiting http://.de

Please check you rebuilt correctly and that you don't have some preferences set, eventually you can use ./mach run --temp-profile to run in a clean profile.

maybe I'm not rebuilding correctly. I used "./mach build"
I actually commented out everything in the URIFixup.jsm module and rebuilt but nothing changed when I ran. no effect at all

Comment 17

[Tanju Brunostar]

Attachment: Bug 1724473 - Added an if statement at the bailout, enabling it to only work when the url starts with an alphanumeric. r?Marco Bonardo <[e-mail]>

Comment 18

[Marco Bonardo [:mak]]

(In reply to Tanju Brunostar from comment #16)

maybe I'm not rebuilding correctly. I used "./mach build"

Yes, it's possible, check the output does not contain any errors, then use ./mach run to execute the build.

Comment 19

[Tanju Brunostar]

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

(In reply to Tanju Brunostar from comment #16)

maybe I'm not rebuilding correctly. I used "./mach build"

Yes, it's possible, check the output does not contain any errors, then use ./mach run to execute the build.

The output contains no errors. It's successful but when I mach run, my changes seem to have no effect

Comment 20

[Marco Bonardo [:mak]]

IIRC you said on Matrix the problem is solved, thus clearing the request.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c5f8ef6e077
Added an if statement at the bailout, enabling it to only work when the url starts with an alphanumeric.  r=mak Bonardo <[e-mail]>

Comment 22

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

https://hg.mozilla.org/mozilla-central/rev/9c5f8ef6e077