Index: components/url_formatter/url_formatter.cc |
diff --git a/components/url_formatter/url_formatter.cc b/components/url_formatter/url_formatter.cc |
index d54b67913ce76d400907266963475bae6920cdd9..0bd994a92461497b9d59cff6766aca3b13ef84ad 100644 |
--- a/components/url_formatter/url_formatter.cc |
+++ b/components/url_formatter/url_formatter.cc |
@@ -6,21 +6,26 @@ |
#include <algorithm> |
#include <utility> |
+#include <vector> |
#include "base/lazy_instance.h" |
#include "base/macros.h" |
#include "base/numerics/safe_conversions.h" |
#include "base/strings/string_piece.h" |
+#include "base/strings/string_split.h" |
#include "base/strings/string_util.h" |
#include "base/strings/utf_offset_string_conversions.h" |
#include "base/strings/utf_string_conversions.h" |
#include "base/threading/thread_local_storage.h" |
+#include "net/base/lookup_string_in_fixed_set.h" |
#include "third_party/icu/source/common/unicode/schriter.h" |
#include "third_party/icu/source/common/unicode/uidna.h" |
#include "third_party/icu/source/common/unicode/uniset.h" |
#include "third_party/icu/source/common/unicode/uscript.h" |
+#include "third_party/icu/source/common/unicode/utypes.h" |
#include "third_party/icu/source/common/unicode/uvernum.h" |
#include "third_party/icu/source/i18n/unicode/regex.h" |
+#include "third_party/icu/source/i18n/unicode/translit.h" |
#include "third_party/icu/source/i18n/unicode/uspoof.h" |
#include "url/gurl.h" |
#include "url/third_party/mozilla/url_parse.h" |
@@ -191,6 +196,56 @@ base::string16 FormatViewSourceUrl( |
return result; |
} |
+// A helper class for IDN Spoof checking, used to ensure that no IDN input is |
+// spoofable per Chromium's standard of spoofability. For a more thorough |
+// explanation of how spoof checking works in Chromium, see |
+// http://dev.chromium.org/developers/design-documents/idn-in-google-chrome . |
+class IDNSpoofChecker { |
Peter Kasting
2017/05/09 01:37:03
Nit: It might be nice to pull this class out to it
jungshik at Google
2017/05/10 18:05:13
Ok. pulled it out.
|
+ public: |
+ IDNSpoofChecker(); |
+ |
+ // Returns true if |label| is safe to display as Unicode. When the TLD is |
Peter Kasting
2017/05/09 01:37:03
Nit: Does the second sentence here really need to
jungshik at Google
2017/05/10 18:05:13
Yeah, that's better.
|
+ // ASCII, check if a label is entirely made of Cyrillic letters that look like |
+ // Latin letters. In the event of library failure, all IDN inputs will be |
+ // treated as unsafe. |
+ bool Check(base::StringPiece16 label, bool is_tld_ascii); |
Peter Kasting
2017/05/09 01:37:03
Nit: This is a poor function name; how about somet
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ |
+ // Returns true if |hostname| or the last few components of |hostname| looks |
+ // similar to one of top domains listed in top_domains/alexa_domains.list. Two |
+ // checks are done: |
+ // 1. Calculate the skeleton of |hostname| based on the Unicode confusable |
+ // character list and look it up in the pre-calculated skeleton list of |
+ // top domains. |
+ // 2. Look up the diacritic-free version of |hostname| in the list of |
+ // top domains. Note that non-IDN hostnames will not get here. |
+ bool SimilarToTopDomains(base::StringPiece16 hostname); |
+ |
+ private: |
+ void SetAllowedUnicodeSet(UErrorCode* status); |
Peter Kasting
2017/05/09 01:37:03
Nit: I suggest adding comments for these even thou
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ bool IsMadeOfLatinAlikeCyrillic(const icu::UnicodeString& label_string); |
+ bool GetSkeleton(base::StringPiece16 hostname, std::string* skeleton); |
+ |
+ USpoofChecker* checker_; |
+ icu::UnicodeSet deviation_characters_; |
+ icu::UnicodeSet non_ascii_latin_letters_; |
+ icu::UnicodeSet kana_letters_exceptions_; |
+ icu::UnicodeSet combining_diacritics_exceptions_; |
+ icu::UnicodeSet cyrillic_letters_; |
+ icu::UnicodeSet cyrillic_letters_latin_alike_; |
+ icu::UnicodeSet lgc_letters_n_ascii_; |
+ icu::Transliterator* transliterator_; |
+ |
+ DISALLOW_COPY_AND_ASSIGN(IDNSpoofChecker); |
+}; |
+ |
+base::LazyInstance<IDNSpoofChecker>::Leaky g_idn_spoof_checker = |
+ LAZY_INSTANCE_INITIALIZER; |
+base::ThreadLocalStorage::StaticSlot tls_index = TLS_INITIALIZER; |
+ |
+void OnThreadTermination(void* regex_matcher) { |
Peter Kasting
2017/05/09 01:37:03
Let me guess: the RegexMatcher uses internal state
|
+ delete reinterpret_cast<icu::RegexMatcher*>(regex_matcher); |
+} |
+ |
// TODO(brettw): We may want to skip this step in the case of file URLs to |
// allow unicode UNC hostnames regardless of encodings. |
base::string16 IDNToUnicodeWithAdjustments( |
@@ -212,6 +267,7 @@ base::string16 IDNToUnicodeWithAdjustments( |
// Do each component of the host separately, since we enforce script matching |
// on a per-component basis. |
base::string16 out16; |
+ bool has_idn_component = false; |
Peter Kasting
2017/05/09 01:37:03
Can we reach this function with an input that does
jungshik at Google
2017/05/10 18:05:13
IDNToUnicode (which calls this function) is called
|
for (size_t component_start = 0, component_end; |
component_start < input16.length(); |
component_start = component_end + 1) { |
@@ -227,6 +283,7 @@ base::string16 IDNToUnicodeWithAdjustments( |
converted_idn = |
IDNToUnicodeOneComponent(input16.data() + component_start, |
component_length, is_tld_ascii, &out16); |
+ has_idn_component = has_idn_component || converted_idn; |
Peter Kasting
2017/05/09 01:37:03
Nit: Or use |=
jungshik at Google
2017/05/10 18:05:13
Changed.
|
} |
size_t new_component_length = out16.length() - new_component_start; |
@@ -239,43 +296,14 @@ base::string16 IDNToUnicodeWithAdjustments( |
if (component_end < input16.length()) |
out16.push_back('.'); |
} |
- return out16; |
-} |
- |
-// A helper class for IDN Spoof checking, used to ensure that no IDN input is |
-// spoofable per Chromium's standard of spoofability. For a more thorough |
-// explanation of how spoof checking works in Chromium, see |
-// http://dev.chromium.org/developers/design-documents/idn-in-google-chrome . |
-class IDNSpoofChecker { |
- public: |
- IDNSpoofChecker(); |
- |
- // Returns true if |label| is safe to display as Unicode. When the TLD is |
- // ASCII, check if a label is entirely made of Cyrillic letters that look like |
- // Latin letters. In the event of library failure, all IDN inputs will be |
- // treated as unsafe. |
- bool Check(base::StringPiece16 label, bool is_tld_ascii); |
- |
- private: |
- void SetAllowedUnicodeSet(UErrorCode* status); |
- bool IsMadeOfLatinAlikeCyrillic(const icu::UnicodeString& label_string); |
- |
- USpoofChecker* checker_; |
- icu::UnicodeSet deviation_characters_; |
- icu::UnicodeSet non_ascii_latin_letters_; |
- icu::UnicodeSet kana_letters_exceptions_; |
- icu::UnicodeSet cyrillic_letters_; |
- icu::UnicodeSet cyrillic_letters_latin_alike_; |
- |
- DISALLOW_COPY_AND_ASSIGN(IDNSpoofChecker); |
-}; |
- |
-base::LazyInstance<IDNSpoofChecker>::Leaky g_idn_spoof_checker = |
- LAZY_INSTANCE_INITIALIZER; |
-base::ThreadLocalStorage::StaticSlot tls_index = TLS_INITIALIZER; |
-void OnThreadTermination(void* regex_matcher) { |
- delete reinterpret_cast<icu::RegexMatcher*>(regex_matcher); |
+ if (has_idn_component && |
Peter Kasting
2017/05/09 01:37:03
Nit: Might want a comment above this block like "L
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ g_idn_spoof_checker.Get().SimilarToTopDomains(out16)) { |
+ if (adjustments) |
+ adjustments->clear(); |
+ return input16; |
+ } |
Peter Kasting
2017/05/09 01:37:03
Nit: Blank line after this?
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ return out16; |
} |
IDNSpoofChecker::IDNSpoofChecker() { |
@@ -323,11 +351,14 @@ IDNSpoofChecker::IDNSpoofChecker() { |
UNICODE_STRING_SIMPLE("[[:Latin:] - [a-zA-Z]]"), status); |
non_ascii_latin_letters_.freeze(); |
- // These letters are parts of |dangerous_patterns_|. |
+ // The following two sets are parts of |dangerous_patterns_|. |
kana_letters_exceptions_ = icu::UnicodeSet( |
UNICODE_STRING_SIMPLE("[\\u3078-\\u307a\\u30d8-\\u30da\\u30fb-\\u30fe]"), |
status); |
kana_letters_exceptions_.freeze(); |
+ combining_diacritics_exceptions_ = icu::UnicodeSet( |
+ UNICODE_STRING_SIMPLE("[\\u0300-\\u0339]"), status); |
+ combining_diacritics_exceptions_.freeze(); |
// These Cyrillic letters look like Latin. A domain label entirely made of |
// these letters is blocked as a simplified whole-script-spoofable. |
@@ -339,7 +370,37 @@ IDNSpoofChecker::IDNSpoofChecker() { |
icu::UnicodeSet(UNICODE_STRING_SIMPLE("[[:Cyrl:]]"), status); |
cyrillic_letters_.freeze(); |
- DCHECK(U_SUCCESS(status)); |
+ // This set is used to determine whether or not to apply a slow |
+ // transliteration to remove diacritics to a given hostname before the |
+ // confusable skeleton calculation for comparison with top domain names. If |
+ // it has any character outside the set, the expensive step will be skipped |
+ // because it cannot match any of top domain names. |
+ // The last ([\u0300-\u0339] is a shorthand for "[:Identifier_Status=Allowed:] |
+ // & [:Script_Extensions=Inherited:] - [\\u200C\\u200D]". The latter is a |
+ // subset of the former but it does not matter because hostnames with |
+ // characters outside the latter set would be rejected in an earlier step. |
+ lgc_letters_n_ascii_ = icu::UnicodeSet(UNICODE_STRING_SIMPLE( |
+ "[[:Latin:][:Greek:][:Cyrillic:][0-9\\u002e_\\u002d][\\u0300-\\u0339]]"), |
+ status); |
+ lgc_letters_n_ascii_.freeze(); |
+ |
+ // Used for diacritics-removal before the skeleton calculation. Add |
+ // "ł > l; ø > o; đ > d" that are not handled by "NFD; Nonspacing mark |
+ // removal; NFC". On top of that, supplement the Unicode confusable list by |
+ // replacing {U+043A (к), U+0138(ĸ), U+03BA(κ)}, U+04CF (ӏ) and U+043F(п) by |
+ // 'k', 'l' and 'n', respectively. |
+ // TODO(jshin): Revisit "ł > l; ø > o" mapping. |
Peter Kasting
2017/05/09 01:37:03
Nit: Might want to link this TODO to a bug or othe
|
+ UParseError parse_error; |
+ transliterator_ = icu::Transliterator::createFromRules( |
+ UNICODE_STRING_SIMPLE("DropAcc"), |
+ icu::UnicodeString("::NFD; ::[:Nonspacing Mark:] Remove; ::NFC;" |
+ " ł > l; ø > o; đ > d; ӏ > l; [кĸκ] > k; п > n;"), |
+ UTRANS_FORWARD, parse_error, status); |
+ DCHECK(U_SUCCESS(status)) |
+ << "Spoofchecker initalization failed due to an error: " |
+ << u_errorName(status); |
+ if (U_FAILURE(status)) |
Peter Kasting
2017/05/09 01:37:03
Do not handle DCHECK failure; assume DCHECKs canno
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ transliterator_ = nullptr; |
} |
bool IDNSpoofChecker::Check(base::StringPiece16 label, bool is_tld_ascii) { |
@@ -370,23 +431,31 @@ bool IDNSpoofChecker::Check(base::StringPiece16 label, bool is_tld_ascii) { |
return false; |
// If there's no script mixing, the input is regarded as safe without any |
- // extra check unless it contains Kana letter exceptions or it's made entirely |
- // of Cyrillic letters that look like Latin letters. Note that the following |
- // combinations of scripts are treated as a 'logical' single script. |
+ // extra check unless it falls into one of three categories: |
+ // - contains Kana letter exceptions |
+ // - it's made entirely of Cyrillic letters that look like Latin letters. |
Peter Kasting
2017/05/09 01:37:03
Nit: it's -> the TLD is ASCII, and the input is ?
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ // - it has combining diacritic marks. |
+ // Note that the following combinations of scripts are treated as a 'logical' |
+ // single script. |
// - Chinese: Han, Bopomofo, Common |
// - Japanese: Han, Hiragana, Katakana, Common |
// - Korean: Hangul, Han, Common |
result &= USPOOF_RESTRICTION_LEVEL_MASK; |
if (result == USPOOF_ASCII) return true; |
if (result == USPOOF_SINGLE_SCRIPT_RESTRICTIVE && |
- kana_letters_exceptions_.containsNone(label_string)) { |
+ kana_letters_exceptions_.containsNone(label_string) && |
+ combining_diacritics_exceptions_.containsNone(label_string)) { |
// Check Cyrillic confusable only for ASCII TLDs. |
return !is_tld_ascii || !IsMadeOfLatinAlikeCyrillic(label_string); |
} |
// Additional checks for |label| with multiple scripts, one of which is Latin. |
// Disallow non-ASCII Latin letters to mix with a non-Latin script. |
- if (non_ascii_latin_letters_.containsSome(label_string)) |
+ // Note that non-ASCII Latin check should not be applied when the entire label |
Peter Kasting
2017/05/09 01:37:03
Nit: that -> that the ?
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ // is made of Latin. Checking with lgc_letters set here should be fine because |
+ // script mixing of LGC is already rejected. |
+ if (non_ascii_latin_letters_.containsSome(label_string) && |
+ !lgc_letters_n_ascii_.containsAll(label_string)) |
return false; |
if (!tls_index.initialized()) |
@@ -415,6 +484,9 @@ bool IDNSpoofChecker::Check(base::StringPiece16 label, bool is_tld_ascii) { |
// - Disallow U+0585 (Armenian Small Letter Oh) and U+0581 (Armenian Small |
// Letter Co) to be next to Latin. |
// - Disallow Latin 'o' and 'g' next to Armenian. |
+ // - Disallow combining diacritical mark (U+0300-U+0339) after a non-LGC |
+ // character. Other combining diacritical marks are not in the allowed |
+ // character set. |
dangerous_pattern = new icu::RegexMatcher( |
icu::UnicodeString( |
"[^\\p{scx=kana}\\p{scx=hira}\\p{scx=hani}]" |
@@ -428,7 +500,8 @@ bool IDNSpoofChecker::Check(base::StringPiece16 label, bool is_tld_ascii) { |
"^[\\u0585\\u0581]+[a-z]|[a-z][\\u0585\\u0581]+$|" |
"[a-z][\\u0585\\u0581]+[a-z]|" |
"^[og]+[\\p{scx=armn}]|[\\p{scx=armn}][og]+$|" |
- "[\\p{scx=armn}][og]+[\\p{scx=armn}]", |
+ "[\\p{scx=armn}][og]+[\\p{scx=armn}]|" |
+ "[^\\p{scx=latn}\\p{scx=grek}\\p{scx=cyrl}][\\u0300-\\u0339]", |
-1, US_INV), |
0, status); |
tls_index.Set(dangerous_pattern); |
@@ -437,6 +510,59 @@ bool IDNSpoofChecker::Check(base::StringPiece16 label, bool is_tld_ascii) { |
return !dangerous_pattern->find(); |
} |
+#include "components/url_formatter/top_domains/alexa_skeletons-inc.cc" |
+// All the domains in the above file have 3 or fewer labels. |
+const size_t kNumberOfLabelsToCheck = 3; |
Peter Kasting
2017/05/09 01:37:03
Can we write this value into the file so we don't
jungshik at Google
2017/05/10 18:05:13
make_top_domain_gperf can write that out to anoth
Peter Kasting
2017/05/10 22:38:46
Can't we write it to the same file? Having a sepa
|
+ |
+bool LookupStringInSet(base::StringPiece needle, |
Peter Kasting
2017/05/09 01:37:04
Nit: If you're not going to use boring names for y
jungshik at Google
2017/05/10 18:05:13
Inlined it.
|
+ const unsigned char* fixed_set, |
+ size_t set_len) { |
+ return net::LookupStringInFixedSet(fixed_set, set_len, needle.data(), |
+ needle.length()) != net::kDafsaNotFound; |
+} |
+ |
+bool LookupMatchInTopDomains(base::StringPiece hostname) { |
+ // When 'hostname' is a skeleton instead of actual hostname, it's assumed |
+ // that no character other than '.' among those allowed in IDN will have |
+ // '.' as its skeleton. |
+ DCHECK(hostname[hostname.length() - 1] != '.'); |
Peter Kasting
2017/05/09 01:37:03
Nit: hostname.back()
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ auto labels = base::SplitStringPiece(hostname, ".", base::KEEP_WHITESPACE, |
+ base::SPLIT_WANT_ALL); |
+ |
+ while (labels.size() > kNumberOfLabelsToCheck) |
+ labels.erase(labels.begin()); |
Peter Kasting
2017/05/09 01:37:04
Nit: Seems like a single call to vector::erase cou
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ |
+ while (labels.size() > 1) { |
Peter Kasting
2017/05/09 01:37:03
Is this naive loop faster than computing the actua
jungshik at Google
2017/05/10 18:05:13
'hostname' is not a good name (at one point, it's
|
+ std::string partial_hostname = base::JoinString(labels, "."); |
+ if (LookupStringInSet(partial_hostname, kDafsa, arraysize(kDafsa))) |
+ return true; |
+ labels.erase(labels.begin()); |
+ } |
+ return false; |
+} |
+ |
+bool IDNSpoofChecker::SimilarToTopDomains(base::StringPiece16 hostname) { |
+ size_t hostname_length = hostname.length() - |
+ (*(hostname.rbegin()) == '.' ? 1 : 0); |
Peter Kasting
2017/05/09 01:37:03
Nit: Use .back() instead of *rbegin()
jungshik at Google
2017/05/10 18:05:13
Done.
|
+ icu::UnicodeString ustr_host(FALSE, hostname.data(), hostname_length); |
+ // If input has any characters outside Latin-Greek-Cyrillic and [0-9._-], |
+ // there is no point in getting rid of diacritics because combining marks |
+ // attached to non-LGC characters are already blocked. |
+ if (lgc_letters_n_ascii_.span(ustr_host, 0, USET_SPAN_CONTAINED) == |
+ ustr_host.length() && transliterator_) |
Peter Kasting
2017/05/09 01:37:03
Note that if the DCHECK earlier is assumed not to
jungshik at Google
2017/05/10 18:05:13
removed it.
|
+ transliterator_->transliterate(ustr_host); |
+ |
+ UErrorCode status = U_ZERO_ERROR; |
+ icu::UnicodeString ustr_skeleton; |
+ uspoof_getSkeletonUnicodeString(checker_, 0, /* not used. deprecated. */ |
Peter Kasting
2017/05/09 01:37:03
Nit: If you're going to add /* */ (which I'm not s
jungshik at Google
2017/05/10 18:05:13
ok. just removed it.
|
+ ustr_host, ustr_skeleton, &status); |
+ if (U_FAILURE(status)) |
+ return false; |
+ std::string skeleton; |
+ ustr_skeleton.toUTF8String(skeleton); |
+ return LookupMatchInTopDomains(skeleton); |
+} |
+ |
bool IDNSpoofChecker::IsMadeOfLatinAlikeCyrillic( |
const icu::UnicodeString& label_string) { |
// Collect all the Cyrillic letters in |label_string| and see if they're |