Relax the safe redirect requirement for lookalike URLs

The current lookalike check allows redirects from an IDN site to its non IDN
version if the redirect is to the root of the non-IDN domain. It disallows
any redirect chains longer than 2 URLs including the initial URL.

This is too strict and causes problems when the IDN domain first redirects to
another URL on the same site. As an example, this redirect is blocked:
http://québec[.]ca -> https://québec[.]ca -> https://quebec[.]ca

This CL relaxes the safe redirect check and allows redirect chains longer than 2
as long as the first IDN to non-IDN redirect happens to the root of the non-IDN
domain. In practice, two changes were made:

1. The initial IDN URL can now redirect to arbitrary URLs on the same domain an
   unlimited number of times. This is now OK:
   http://sub1[.]québec[.]ca/path1 -> http://sub2[.]québec[.]ca/path2 ->
   https://sub3[.]québec[.]ca/path3 -> http://quebec[.]ca

2. There can now be an unlimited number of redirects to arbitrary URLs after the
   first redirect from the IDN URL to the root of the non-IDN URL. This is now
   OK:
   http://québec[.]ca/path -> http://quebec[.]ca -> http://somerandomdomain[.]com

We don't necessarily need to implement (2) to fix bug 1015084, but there doesn't
seem to be a good reason to impose that artificial restriction either.

Bug: 1015084
Change-Id: Icc760cb171c7e314609e214b9ee8563e4f320014
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881354Reviewed-by: Joe DeBlasio <jdeblasio@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Auto-Submit: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710535}

chrome/browser/lookalikes/lookalike_url_navigation_throttle.cc

@@ -173,6 +173,21 @@ void RecordUMAFromMatchType(MatchType match_type) {
   }
 }
 
+// Returns the index of the first URL in the redirect chain which has a
+// different eTLD+1 than the initial URL. If all URLs have the same eTLD+1,
+// returns 0.
+size_t FindFirstCrossSiteURL(const std::vector<GURL>& redirect_chain) {
+  DCHECK_GE(redirect_chain.size(), 2u);
+  const GURL initial_url = redirect_chain[0];
+  const std::string initial_etld_plus_one = GetETLDPlusOne(initial_url.host());
+  for (size_t i = 1; i < redirect_chain.size(); i++) {
+    if (initial_etld_plus_one != GetETLDPlusOne(redirect_chain[i].host())) {
+      return i;
+    }
+  }
+  return 0;
+}
+
 }  // namespace
 
 // static
@@ -235,10 +250,21 @@ bool IsEditDistanceAtMostOne(const base::string16& str1,
 
 bool IsSafeRedirect(const std::string& matching_domain,
                     const std::vector<GURL>& redirect_chain) {
-  if (redirect_chain.size() != 2) {
+  if (redirect_chain.size() < 2) {
     return false;
   }
-  const GURL redirect_target = redirect_chain[redirect_chain.size() - 1];
+  const size_t first_cross_site_redirect =
+      FindFirstCrossSiteURL(redirect_chain);
+  DCHECK_GE(first_cross_site_redirect, 0u);
+  DCHECK_LE(first_cross_site_redirect, redirect_chain.size() - 1);
+  if (first_cross_site_redirect == 0) {
+    // All URLs in the redirect chain belong to the same eTLD+1.
+    return true;
+  }
+  // There is a redirect from the initial eTLD+1 to another site. In order to be
+  // a safe redirect, it should be to the root of |matching_domain|. This
+  // ignores any further redirects after |matching_domain|.
+  const GURL redirect_target = redirect_chain[first_cross_site_redirect];
   return matching_domain == GetETLDPlusOne(redirect_target.host()) &&
          redirect_target == redirect_target.GetWithEmptyPath();
 }

chrome/browser/lookalikes/lookalike_url_navigation_throttle_unittest.cc

@@ -73,8 +73,11 @@ TEST(LookalikeUrlNavigationThrottleTest, IsEditDistanceAtMostOne) {
 // - http[s]://sité.test/path -> http[s]://site.test
 // - http[s]://subdomain.sité.test -> http[s]://site.test
 // - http[s]://random.test -> http[s]://sité.test -> http[s]://site.test
+// - http://sité.test/path -> https://sité.test/path -> https://site.test ->
+// <any_url>
 // - "subdomain" on either side.
-// This is not safe:
+//
+// These are not safe:
 // - http[s]://[subdomain.]sité.test -> http[s]://[subdomain.]site.test/path
 // because the redirected URL has a path.
 TEST(LookalikeUrlNavigationThrottleTest, IsSafeRedirect) {
@@ -85,6 +88,25 @@ TEST(LookalikeUrlNavigationThrottleTest, IsSafeRedirect) {
   EXPECT_TRUE(IsSafeRedirect(
       "example.com",
       {GURL("http://éxample.com"), GURL("http://subdomain.example.com")}));
+  EXPECT_TRUE(IsSafeRedirect(
+      "example.com", {GURL("http://éxample.com"), GURL("http://example.com"),
+                      GURL("https://example.com")}));
+  // Original site redirects to HTTPS.
+  EXPECT_TRUE(IsSafeRedirect(
+      "example.com", {GURL("http://éxample.com"), GURL("https://éxample.com"),
+                      GURL("https://example.com")}));
+  // Original site redirects to HTTPS which redirects to HTTP which redirects
+  // back to HTTPS of the non-IDN version.
+  EXPECT_TRUE(IsSafeRedirect(
+      "example.com",
+      {GURL("http://éxample.com/redir1"), GURL("https://éxample.com/redir1"),
+       GURL("http://éxample.com/redir2"), GURL("https://example.com/")}));
+  // Same as above, but there is another redirect at the end of the chain.
+  EXPECT_TRUE(IsSafeRedirect(
+      "example.com",
+      {GURL("http://éxample.com/redir1"), GURL("https://éxample.com/redir1"),
+       GURL("http://éxample.com/redir2"), GURL("https://example.com/"),
+       GURL("https://totallydifferentsite.com/somepath")}));
 
   // Not a redirect, the chain is too short.
   EXPECT_FALSE(IsSafeRedirect("example.com", {GURL("http://éxample.com")}));
@@ -95,7 +117,8 @@ TEST(LookalikeUrlNavigationThrottleTest, IsSafeRedirect) {
   EXPECT_FALSE(IsSafeRedirect(
       "example.com",
       {GURL("http://éxample.com"), GURL("http://example.com/path")}));
-  // Not safe: The chain is too long.
+  // Not safe: The first redirect away from éxample.com is not to the matching
+  // non-IDN site.
   EXPECT_FALSE(IsSafeRedirect("example.com", {GURL("http://éxample.com"),
                                               GURL("http://intermediate.com"),
                                               GURL("http://example.com")}));