Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(994)

Issue 2897873002: Add checks against spoofing attempt at top domains (Closed)

Created:
3 years, 7 months ago by jungshik at Google
Modified:
3 years, 7 months ago
Reviewers:
Peter Kasting
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add checks against spoofing attempt at top domains Original CL (https://codereview.chromium.org/2784933002) was reverted due to a compile failure on win_x64 (not detected by CQ but detected post-landing). That issue was addressed using checked_cast. Remove diacritic marks from a hostname and calculate the confusability skeleton of the accent-free name. Look it up in the pre-calculated list of the skeletons of top 10k domains. Removing diacritic marks from a hostname is equivalent to comparing names with the primary collation strength in the root locale. To make them equivalent, three mappings are added (ł > l; ø > o; đ > d) on top of the diacritic-removal. Also add two more mappings ([кĸκ] > k, п > n) to supplement the Unicode's confusables list. Binary file size increase: ~ 59kB for the DAFSA representation of top domain name skeletons. The IDN display policy check takes ~ 2µs longer on the average (3.3 µs => 5.5µs) on my machine per the test run over ~1 million IDNs in com TLD). It adds about 1500 domains to the list of domains to display in Punycode out of ~ 1 million IDNs in com TLD. (3018 => 4571) In addition, disallow combining diarctic marks unless they're preceded by Latin-Greek-Cyrillic. TBR=pkasting@chromium.org BUG=703750, 714628, 719199, 722639 TEST=components_unittests --gtest_filter=*IDNToUni* CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_chromium_x64_rel_ng,win10_chromium_x64_rel_ng Review-Url: https://codereview.chromium.org/2897873002 Cr-Commit-Position: refs/heads/master@{#473519} Committed: https://chromium.googlesource.com/chromium/src/+/a586e96794b89bef4729b33369b8c2035564d376

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18752 lines, -13 lines) Patch
M components/url_formatter/BUILD.gn View 2 chunks +2 lines, -0 lines 0 comments Download
M components/url_formatter/idn_spoof_checker.h View 3 chunks +15 lines, -0 lines 0 comments Download
M components/url_formatter/idn_spoof_checker.cc View 9 chunks +104 lines, -8 lines 0 comments Download
A components/url_formatter/top_domains/BUILD.gn View 1 chunk +32 lines, -0 lines 0 comments Download
A components/url_formatter/top_domains/README View 1 chunk +16 lines, -0 lines 0 comments Download
A components/url_formatter/top_domains/alexa_domains.list View 1 chunk +9176 lines, -0 lines 0 comments Download
A components/url_formatter/top_domains/alexa_skeletons.gperf View 1 chunk +9186 lines, -0 lines 0 comments Download
A components/url_formatter/top_domains/make_alexa_top_list.py View 1 chunk +55 lines, -0 lines 0 comments Download
A components/url_formatter/top_domains/make_top_domain_gperf.cc View 1 chunk +122 lines, -0 lines 0 comments Download
M components/url_formatter/url_formatter.cc View 3 chunks +11 lines, -0 lines 0 comments Download
M components/url_formatter/url_formatter_unittest.cc View 4 chunks +33 lines, -5 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
jungshik at Google
Going with TBR because the change wrt the previous CL is trivial (adding checked_cast<> for ...
3 years, 7 months ago (2017-05-22 05:23:41 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2897873002/1
3 years, 7 months ago (2017-05-22 05:24:12 UTC) #3
jungshik at Google
On 2017/05/22 05:23:41, jungshik at Google wrote: > Going with TBR because the change wrt ...
3 years, 7 months ago (2017-05-22 06:27:22 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/a586e96794b89bef4729b33369b8c2035564d376
3 years, 7 months ago (2017-05-22 07:20:34 UTC) #7
Peter Kasting
3 years, 7 months ago (2017-05-22 17:28:56 UTC) #8
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698