SafeBrowsing: Do not perform real-time URL checks on private URLs.

Since private URLs can't really be checked for safety, there's no
benefit of checking their reputation with the server.

R=bdea

Bug: 963165
Change-Id: I927fa1fa82dd491cbb2171fa1c9d0ee6a80f0dd7
Fixed: 1021671
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900256
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Auto-Submit: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712856}

components/safe_browsing/browser/safe_browsing_url_checker_impl.cc

@@ -298,12 +298,7 @@ void SafeBrowsingUrlCheckerImpl::ProcessUrls() {
                  &SafeBrowsingUrlCheckerImpl::OnTimeout);
 
     bool safe_synchronously;
-    auto* rt_lookup_service = database_manager_->GetRealTimeUrlLookupService();
-    if (real_time_lookup_enabled_ &&
-        RealTimePolicyEngine::CanPerformFullURLLookupForResourceType(
-            resource_type_) &&
-        rt_lookup_service && rt_lookup_service->CanCheckUrl(url) &&
-        !rt_lookup_service->IsInBackoffMode()) {
+    if (CanPerformFullURLLookup(url)) {
       UMA_HISTOGRAM_ENUMERATION("SafeBrowsing.RT.ResourceTypes.Checked",
                                 resource_type_);
       safe_synchronously = false;
@@ -379,6 +374,19 @@ void SafeBrowsingUrlCheckerImpl::BlockAndProcessUrls(bool showed_interstitial) {
   }
 }
 
+bool SafeBrowsingUrlCheckerImpl::CanPerformFullURLLookup(const GURL& url) {
+  if (!real_time_lookup_enabled_)
+    return false;
+
+  if (!RealTimePolicyEngine::CanPerformFullURLLookupForResourceType(
+          resource_type_))
+    return false;
+
+  auto* rt_lookup_service = database_manager_->GetRealTimeUrlLookupService();
+  return rt_lookup_service && rt_lookup_service->CanCheckUrl(url) &&
+         !rt_lookup_service->IsInBackoffMode();
+}
+
 void SafeBrowsingUrlCheckerImpl::OnBlockingPageComplete(bool proceed) {
   DCHECK_EQ(STATE_DISPLAYING_BLOCKING_PAGE, state_);
 

components/safe_browsing/browser/safe_browsing_url_checker_impl.h

@@ -135,6 +135,10 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
 
   void OnBlockingPageComplete(bool proceed);
 
+  // Helper method that checks whether |url|'s reputation can be checked using
+  // real time lookups.
+  bool CanPerformFullURLLookup(const GURL& url);
+
   SBThreatType CheckWebUIUrls(const GURL& url);
 
   // Returns false if this object has been destroyed by the callback. In that

components/safe_browsing/realtime/url_lookup_service.cc

@@ -5,10 +5,13 @@
 #include "components/safe_browsing/realtime/url_lookup_service.h"
 
 #include "base/base64url.h"
+#include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "components/safe_browsing/db/v4_protocol_manager_util.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/base/ip_address.h"
 #include "net/base/load_flags.h"
+#include "net/base/url_util.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/resource_request.h"
@@ -147,7 +150,23 @@ void RealTimeUrlLookupService::OnURLLoaderComplete(
 }
 
 bool RealTimeUrlLookupService::CanCheckUrl(const GURL& url) const {
-  return url.SchemeIsHTTPOrHTTPS();
+  if (!url.SchemeIsHTTPOrHTTPS()) {
+    return false;
+  }
+
+  if (net::IsLocalhost(url)) {
+    // Includes: "//localhost/", "//localhost.localdomain/", "//127.0.0.1/"
+    return false;
+  }
+
+  net::IPAddress ip_address;
+  if (url.HostIsIPAddress() && ip_address.AssignFromIPLiteral(url.host()) &&
+      !ip_address.IsPubliclyRoutable()) {
+    // Includes: "//192.168.1.1/", "//172.16.2.2/", "//10.1.1.1/"
+    return false;
+  }
+
+  return true;
 }
 
 std::unique_ptr<RTLookupRequest> RealTimeUrlLookupService::FillRequestProto(

components/safe_browsing/realtime/url_lookup_service_unittest.cc

@@ -28,6 +28,7 @@ class RealTimeUrlLookupServiceTest : public PlatformTest {
         std::make_unique<RealTimeUrlLookupService>(test_shared_loader_factory_);
   }
 
+  bool CanCheckUrl(const GURL& url) { return rt_service_->CanCheckUrl(url); }
   void HandleLookupError() { rt_service_->HandleLookupError(); }
   void HandleLookupSuccess() { rt_service_->HandleLookupSuccess(); }
   bool IsInBackoffMode() { return rt_service_->IsInBackoffMode(); }
@@ -143,4 +144,26 @@ TEST_F(RealTimeUrlLookupServiceTest, TestGetSBThreatTypeForRTThreatType) {
                 RTLookupResponse::ThreatInfo::UNCLEAR_BILLING));
 }
 
+TEST_F(RealTimeUrlLookupServiceTest, TestCanCheckUrl) {
+  struct CanCheckUrlCases {
+    const char* url;
+    bool can_check;
+  } can_check_url_cases[] = {{"ftp://example.test/path", false},
+                             {"http://localhost/path", false},
+                             {"http://localhost.localdomain/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.1/path", false},
+                             {"http://10.1.1.1.1/path", true},
+                             {"http://example.test/path", true},
+                             {"https://example.test/path", true}};
+  for (size_t i = 0; i < base::size(can_check_url_cases); i++) {
+    GURL url(can_check_url_cases[i].url);
+    bool expected_can_check = can_check_url_cases[i].can_check;
+    EXPECT_EQ(expected_can_check, CanCheckUrl(url));
+  }
+}
+
 }  // namespace safe_browsing