Commit 3035169b authored by Mustafa Emre Acer's avatar Mustafa Emre Acer Committed by Commit Bot

Fix handling of same-skeleton domains in IDN spoof checker

A previous fix for bug 898343 attempted to handle skeletons that are the same as their domains (e.g. test[.]net).

The fix introduced an early return which caused skeleton lookups to incorrectly match subtrings:
If a domain name contained a top domain as its prefix, spoof checker assumed a skeleton match without checking if it reached the end of the search string (i.e. the skeleton being looked up).
E.g. Skeleton of atést[.]net (atest[.]net) matched the skeleton of test[.]net (test[.]net).

This CL properly checks that all of the search string is consumed before returning a match.

Bug: 925199
Change-Id: Ia548ea686915463e0f93e6bce1bad5721f324821
Reviewed-on: https://chromium-review.googlesource.com/c/1437716
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626867}
parent a0202087
...@@ -35,17 +35,14 @@ class TopDomainPreloadDecoder : public net::extras::PreloadDecoder { ...@@ -35,17 +35,14 @@ class TopDomainPreloadDecoder : public net::extras::PreloadDecoder {
if (!reader->Next(&is_same_skeleton)) if (!reader->Next(&is_same_skeleton))
return false; return false;
std::string top_domain;
if (is_same_skeleton) { if (is_same_skeleton) {
*out_found = true; top_domain = search;
result_ = search; } else {
return true;
}
bool has_com_suffix = false; bool has_com_suffix = false;
if (!reader->Next(&has_com_suffix)) if (!reader->Next(&has_com_suffix))
return false; return false;
std::string top_domain;
for (char c;; top_domain += c) { for (char c;; top_domain += c) {
huffman_decoder().Decode(reader, &c); huffman_decoder().Decode(reader, &c);
if (c == net::extras::PreloadDecoder::kEndOfTable) if (c == net::extras::PreloadDecoder::kEndOfTable)
...@@ -53,6 +50,7 @@ class TopDomainPreloadDecoder : public net::extras::PreloadDecoder { ...@@ -53,6 +50,7 @@ class TopDomainPreloadDecoder : public net::extras::PreloadDecoder {
} }
if (has_com_suffix) if (has_com_suffix)
top_domain += ".com"; top_domain += ".com";
}
if (current_search_offset == 0) { if (current_search_offset == 0) {
*out_found = true; *out_found = true;
......
...@@ -1012,8 +1012,21 @@ const IDNTestCase idn_cases[] = { ...@@ -1012,8 +1012,21 @@ const IDNTestCase idn_cases[] = {
// Test that top domains whose skeletons are the same as the domain name are // Test that top domains whose skeletons are the same as the domain name are
// handled properly. In this case, tést.net should match test.net top // handled properly. In this case, tést.net should match test.net top
// domain. // domain and not be converted to unicode.
{"xn--tst-bma.net", L"t\x00e9st.net", false}, {"xn--tst-bma.net", L"t\x00e9st.net", false},
// Variations of the above, for testing crbug.com/925199.
// some.tést.net should match test.net.
{"some.xn--tst-bma.net", L"some.t\x00e9st.net", false},
// The following should not match test.net, so should be converted to
// unicode.
// ést.net (a suffix of tést.net).
{"xn--st-9ia.net", L"\x00e9st.net", true},
// some.ést.net
{"some.xn--st-9ia.net", L"some.\x00e9st.net", true},
// atést.net (tést.net is a suffix of atést.net)
{"xn--atst-cpa.net", L"at\x00e9st.net", true},
// some.atést.net
{"some.xn--atst-cpa.net", L"some.at\x00e9st.net", true},
// Modifier-letter-voicing should be blocked (wwwˬtest.com). // Modifier-letter-voicing should be blocked (wwwˬtest.com).
{"xn--wwwtest-2be.com", L"www\x02ectest.com", false}, {"xn--wwwtest-2be.com", L"www\x02ectest.com", false},
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment