Skip to content

Commit

Permalink
Reject non-IPv4 hostnames ending in numbers.
Browse files Browse the repository at this point in the history
The URL spec has been updated to parse any hostname ending with a
numeric component as IPv4 unconditionally, with no fallback to
considering it a domain name if it fails to parse as an IPv4 address.
See whatwg/url#619.

This CL updates the url/ parser accordingly.

Bug: 1237032
Change-Id: Ieb0ee3073f65931f97a690b0d852ab9761c328ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3111118
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#915327}
  • Loading branch information
Matt Menke authored and Chromium LUCI CQ committed Aug 25, 2021
1 parent 0cdb72f commit f21b724
Show file tree
Hide file tree
Showing 22 changed files with 908 additions and 1,297 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ TEST_F(RealTimeUrlLookupServiceTest, TestCanCheckUrl) {
{"ftp://example.test/path", false}, {"http://localhost/path", false},
{"http://127.0.0.1/path", false}, {"http://127.0.0.1:2222/path", false},
{"http://192.168.1.1/path", false}, {"http://172.16.2.2/path", false},
{"http://10.1.1.9/path", false}, {"http://10.1.1.09/path", true},
{"http://10.1.1.9/path", false}, {"http://10.1.1.0xG/path", true},
{"http://example.test/path", true}, {"http://nodothost/path", false},
{"http://x.x/shorthost", false}};
for (auto& can_check_url_case : can_check_url_cases) {
Expand Down
45 changes: 0 additions & 45 deletions net/base/schemeful_site_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -364,51 +364,6 @@ TEST(SchemefulSiteTest, ConvertWebSocketToHttp) {
EXPECT_EQ(url::kFileScheme, file_site.GetInternalOriginForTesting().scheme());
}

// Test for a hack to work around https://crbug.com/1157010, until a more
// permanent solution is in place. Purely numeric eTLD+1's can't safely be
// stored in url::Origins. Not only does trying to do so DCHECK, but converting
// them to a GURL, as some code does, results in a URL with an IPv4 domain,
// which is not correct.
TEST(SchemefulSiteTest, NumericEtldPlusOne) {
base::HistogramTester histogram_tester;
SchemefulSite site(url::Origin::Create(GURL("https://foo.127.1/")));
EXPECT_EQ("foo.127.1", site.registrable_domain_or_host_for_testing());
EXPECT_NE(site, SchemefulSite(GURL("https://127.0.0.1/")));

SchemefulSite site2(url::Origin::Create(GURL("https://bar.foo.127.1/")));
EXPECT_EQ("bar.foo.127.1", site2.registrable_domain_or_host_for_testing());
EXPECT_NE(site2, SchemefulSite(GURL("https://127.0.0.1/")));
EXPECT_NE(site, site2);

EXPECT_FALSE(SchemefulSite::CreateIfHasRegisterableDomain(
url::Origin::Create(GURL("https://foo.127.1/"))));
}

TEST(SchemefulSiteTest, SiteDomainIsSafeHistogram) {
base::HistogramTester histogram_tester1;
SchemefulSite site(url::Origin::Create(GURL("https://foo.127.1/")));
histogram_tester1.ExpectUniqueSample("Net.SiteDomainIsSafe", false, 1);

base::HistogramTester histogram_tester2;
SchemefulSite site2(url::Origin::Create(GURL("https://foo.bar.127.1/")));
histogram_tester2.ExpectUniqueSample("Net.SiteDomainIsSafe", false, 1);

base::HistogramTester histogram_tester3;
SchemefulSite site3(url::Origin::Create(GURL("https://127.0.0.1/")));
histogram_tester3.ExpectUniqueSample("Net.SiteDomainIsSafe", true, 1);

base::HistogramTester histogram_tester4;
SchemefulSite site4(url::Origin::Create(GURL("https://foo.test/")));
histogram_tester4.ExpectUniqueSample("Net.SiteDomainIsSafe", true, 1);

base::HistogramTester histogram_tester5;
SchemefulSite site5{url::Origin()};
histogram_tester5.ExpectTotalCount("Net.SiteDomainIsSafe", 0);
SchemefulSite site6(
url::Origin::Create(GURL("data:text/html,<body>Hello World</body>")));
histogram_tester5.ExpectTotalCount("Net.SiteDomainIsSafe", 0);
}

TEST(SchemefulSiteTest, GetGURL) {
struct {
url::Origin origin;
Expand Down
8 changes: 6 additions & 2 deletions net/cert/x509_certificate_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1262,11 +1262,15 @@ const CertificateNameVerifyTestData kNameVerifyTestData[] = {
{true, "FE80::200:f8ff:fe21:67cf", "",
"x00000000000000000000000006070808,xfe800000000000000200f8fffe2167cf,"
"xff0000000000000000000000060708ff,10.0.0.1"},
// Numeric only hostnames (none of these are considered valid IP addresses).
// Invalid hostnames with final numeric component.
{false, "121.2.3.512", "1*1.2.3.512,*1.2.3.512,1*.2.3.512,*.2.3.512",
"121.2.3.0"},
{false, "1.2.3.4.5.6", "*.2.3.4.5.6"},
{true, "1.2.3.4.5", "1.2.3.4.5"},
{false, "1.2.3.4.5", "1.2.3.4.5"},
{false, "a.0.0.1", "*.0.0.1"},
// IP addresses in dNSName should not match commonName
{false, "127.0.0.1", "127.0.0.1"},
{false, "127.0.0.1", "*.0.0.1"},
// Invalid host names.
{false, ".", ""},
{false, ".", "."},
Expand Down
4 changes: 3 additions & 1 deletion net/dns/dns_alias_utility_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ TEST(DnsAliasUtilityTest, SanitizeDnsAliases) {
{"a.com", nullptr},
{"b_o.org", "b_o.org"},
{"alias.com", nullptr},
{"1..3.2", "1..3.2"},
{"1..3.2", nullptr},
{"1.2.3.09", nullptr},
{"foo.4", nullptr},
{"a,b,c", "a%2Cb%2Cc"},
{"f/g", nullptr},
{"www?", nullptr},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
Found 664 tests; 378 PASS, 286 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 664 tests; 381 PASS, 283 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS Parsing: <http://example .
org> against <http://example.org/foo/bar>
Expand Down Expand Up @@ -645,35 +645,35 @@ FAIL Parsing: <abc:rootless> against <abc://host/path> assert_equals: href expec
FAIL Parsing: <abc:rootless> against <abc:/path> assert_equals: href expected "abc:rootless" but got "abc:/rootless"
PASS Parsing: <abc:rootless> against <abc:path>
FAIL Parsing: <abc:/rooted> against <abc://host/path> assert_equals: href expected "abc:/rooted" but got "abc://host/rooted"
FAIL Parsing: <http://1.2.3.4.5> against <http://other.com/> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://1.2.3.4.5.> against <http://other.com/> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://0..0x300/> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://0..0x300./> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://256.256.256.256.256> against <http://other.com/> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://256.256.256.256.256.> against <http://other.com/> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://1.2.3.08> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://1.2.3.08.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://1.2.3.09> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://09.2.3.4> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://09.2.3.4.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://01.2.3.4.5> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://01.2.3.4.5.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://1.2.3.4.5> against <http://other.com/> assert_equals: failure should set href to input expected "http://1.2.3.4.5" but got "http://1.2.3.4.5/"
FAIL Parsing: <http://1.2.3.4.5.> against <http://other.com/> assert_equals: failure should set href to input expected "http://1.2.3.4.5." but got "http://1.2.3.4.5./"
PASS Parsing: <http://0..0x300/> against <about:blank>
PASS Parsing: <http://0..0x300./> against <about:blank>
FAIL Parsing: <http://256.256.256.256.256> against <http://other.com/> assert_equals: failure should set href to input expected "http://256.256.256.256.256" but got "http://256.256.256.256.256/"
FAIL Parsing: <http://256.256.256.256.256.> against <http://other.com/> assert_equals: failure should set href to input expected "http://256.256.256.256.256." but got "http://256.256.256.256.256./"
FAIL Parsing: <http://1.2.3.08> against <about:blank> assert_equals: failure should set href to input expected "http://1.2.3.08" but got "http://1.2.3.08/"
FAIL Parsing: <http://1.2.3.08.> against <about:blank> assert_equals: failure should set href to input expected "http://1.2.3.08." but got "http://1.2.3.08./"
FAIL Parsing: <http://1.2.3.09> against <about:blank> assert_equals: failure should set href to input expected "http://1.2.3.09" but got "http://1.2.3.09/"
FAIL Parsing: <http://09.2.3.4> against <about:blank> assert_equals: failure should set href to input expected "http://09.2.3.4" but got "http://09.2.3.4/"
FAIL Parsing: <http://09.2.3.4.> against <about:blank> assert_equals: failure should set href to input expected "http://09.2.3.4." but got "http://09.2.3.4./"
FAIL Parsing: <http://01.2.3.4.5> against <about:blank> assert_equals: failure should set href to input expected "http://01.2.3.4.5" but got "http://01.2.3.4.5/"
FAIL Parsing: <http://01.2.3.4.5.> against <about:blank> assert_equals: failure should set href to input expected "http://01.2.3.4.5." but got "http://01.2.3.4.5./"
FAIL Parsing: <http://0x100.2.3.4> against <about:blank> assert_equals: failure should set href to input expected "http://0x100.2.3.4" but got "http://0x100.2.3.4/"
FAIL Parsing: <http://0x100.2.3.4.> against <about:blank> assert_equals: failure should set href to input expected "http://0x100.2.3.4." but got "http://0x100.2.3.4./"
FAIL Parsing: <http://0x1.2.3.4.5> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://0x1.2.3.4.5.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.1.2.3.4> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.1.2.3.4.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.2.3.4> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.2.3.4.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.09> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.09.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.0x4> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.0x4.> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://0x1.2.3.4.5> against <about:blank> assert_equals: failure should set href to input expected "http://0x1.2.3.4.5" but got "http://0x1.2.3.4.5/"
FAIL Parsing: <http://0x1.2.3.4.5.> against <about:blank> assert_equals: failure should set href to input expected "http://0x1.2.3.4.5." but got "http://0x1.2.3.4.5./"
FAIL Parsing: <http://foo.1.2.3.4> against <about:blank> assert_equals: failure should set href to input expected "http://foo.1.2.3.4" but got "http://foo.1.2.3.4/"
FAIL Parsing: <http://foo.1.2.3.4.> against <about:blank> assert_equals: failure should set href to input expected "http://foo.1.2.3.4." but got "http://foo.1.2.3.4./"
FAIL Parsing: <http://foo.2.3.4> against <about:blank> assert_equals: failure should set href to input expected "http://foo.2.3.4" but got "http://foo.2.3.4/"
FAIL Parsing: <http://foo.2.3.4.> against <about:blank> assert_equals: failure should set href to input expected "http://foo.2.3.4." but got "http://foo.2.3.4./"
FAIL Parsing: <http://foo.09> against <about:blank> assert_equals: failure should set href to input expected "http://foo.09" but got "http://foo.09/"
FAIL Parsing: <http://foo.09.> against <about:blank> assert_equals: failure should set href to input expected "http://foo.09." but got "http://foo.09./"
FAIL Parsing: <http://foo.0x4> against <about:blank> assert_equals: failure should set href to input expected "http://foo.0x4" but got "http://foo.0x4/"
FAIL Parsing: <http://foo.0x4.> against <about:blank> assert_equals: failure should set href to input expected "http://foo.0x4." but got "http://foo.0x4./"
PASS Parsing: <http://foo.09..> against <about:blank>
FAIL Parsing: <http://0999999999999999999/> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.0x> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://foo.0XFfFfFfFfFfFfFfFfFfAcE123> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
FAIL Parsing: <http://💩.123/> against <about:blank> assert_unreached: Expected URL to fail parsing Reached unreachable code
PASS Parsing: <http://0999999999999999999/> against <about:blank>
FAIL Parsing: <http://foo.0x> against <about:blank> assert_equals: failure should set href to input expected "http://foo.0x" but got "http://foo.0x/"
FAIL Parsing: <http://foo.0XFfFfFfFfFfFfFfFfFfAcE123> against <about:blank> assert_equals: failure should set href to input expected "http://foo.0XFfFfFfFfFfFfFfFfFfAcE123" but got "http://foo.0xfffffffffffffffffface123/"
FAIL Parsing: <http://💩.123/> against <about:blank> assert_equals: failure should set href to input expected "http://💩.123/" but got "http://xn--ls8h.123
Harness: the test ran to completion.

Loading

0 comments on commit f21b724

Please sign in to comment.