[SafeBrowsing] Alternate extensions should also be subject to block list.

chrome/browser/safe_browsing/unverified_download_policy.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/unverified_download_policy.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/unverified_download_field_trial.h"
 #include "chrome/common/safe_browsing/download_protection_util.h"
 #include "components/rappor/rappor_service.h"
 #include "components/rappor/rappor_utils.h"
 #include "components/safe_browsing_db/database_manager.h"
 #include "content/public/browser/browser_thread.h"
 
@@ skipping 33 matching lines @@
   int uma_file_type =
       download_protection_util::GetSBClientDownloadExtensionValueForUMA(file);
   if (policy == UnverifiedDownloadPolicy::ALLOWED) {
     RecordPolicyMetric("SafeBrowsing.UnverifiedDownloads.Allowed",
                        uma_file_type, requestor);
   } else {
     RecordPolicyMetric("SafeBrowsing.UnverifiedDownloads.Blocked",
                        uma_file_type, requestor);
   }
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                                   base::Bind(callback, policy));
+                          base::Bind(callback, policy));
 }
 
 void CheckFieldTrialOnAnyThread(
     const base::FilePath& file,
+    const std::vector<base::FilePath::StringType>& alternate_extensions,
     const GURL& requestor,
     const UnverifiedDownloadCheckCompletionCallback& callback) {
-  bool is_allowed = IsUnverifiedDownloadAllowedByFieldTrial(file);
-  RespondWithPolicy(file, callback, requestor, is_allowed
-                                        ? UnverifiedDownloadPolicy::ALLOWED
-                                        : UnverifiedDownloadPolicy::DISALLOWED);
+  if (!IsUnverifiedDownloadAllowedByFieldTrial(file)) {
+    RespondWithPolicy(file, callback, requestor,
+                      UnverifiedDownloadPolicy::DISALLOWED);
+    return;
+  }
+
+  for (const auto& extension : alternate_extensions) {
+    base::FilePath alternate_filename = file.AddExtension(extension);
+    if (!IsUnverifiedDownloadAllowedByFieldTrial(alternate_filename)) {
+      RespondWithPolicy(alternate_filename, callback, requestor,
+                        UnverifiedDownloadPolicy::DISALLOWED);
+      return;
+    }
+  }
+
+  RespondWithPolicy(file, callback, requestor,
+                    UnverifiedDownloadPolicy::ALLOWED);
 }
 
 void CheckWhitelistOnIOThread(
     scoped_refptr<SafeBrowsingService> service,
     const GURL& requestor,
     const base::FilePath& file,
+    const std::vector<base::FilePath::StringType>& alternate_extensions,
     const UnverifiedDownloadCheckCompletionCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   int uma_file_type =
       download_protection_util::GetSBClientDownloadExtensionValueForUMA(file);
 
   if (!service || !service->enabled()) {
     // If the SafeBrowsing service was disabled, don't try to check against the
     // field trial list. Instead allow the download. We are assuming that if the
     // SafeBrowsing service was disabled for this user, then we shouldn't
     // interefere with unverified downloads.
     RecordPolicyMetric(
         "SafeBrowsing.UnverifiedDownloads.AllowedDueToDisabledService",
         uma_file_type, requestor);
     RespondWithPolicy(file, callback, requestor,
                       UnverifiedDownloadPolicy::ALLOWED);
     return;
   }
 
   if (service->database_manager() &&
       service->database_manager()->MatchDownloadWhitelistUrl(requestor)) {
     RecordPolicyMetric("SafeBrowsing.UnverifiedDownloads.AllowedByWhitelist",
                        uma_file_type, requestor);
     RespondWithPolicy(file, callback, requestor,
                       UnverifiedDownloadPolicy::ALLOWED);
     return;
   }
 
-  CheckFieldTrialOnAnyThread(file, requestor, callback);
+  CheckFieldTrialOnAnyThread(file, alternate_extensions, requestor, callback);
 }
 
 }  // namespace
 
 void CheckUnverifiedDownloadPolicy(
     const GURL& requestor,
     const base::FilePath& file,
+    const std::vector<base::FilePath::StringType>& alternate_extensions,
     const UnverifiedDownloadCheckCompletionCallback& callback) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS(
+      "SafeBrowsing.UnverifiedDownloads.AlternateExtensionCount",
+      alternate_extensions.size() + 1, 1, 31, 32);

[Nathan Parker, 2016/01/21 21:47:43]

Thanks.
I'm curious: Why +1?  Why not 0-32?

[asanka, 2016/01/21 22:08:47]

0 is the underflow bucket. If the minimum is 0, it's silently rounded up to 1.

I've decided to go with a smaller range with fewer bucket and with a linear
bucket distribution.

[Steven Holte, 2016/01/22 23:50:49]

You don't need the +1.  This histogram will get a 0-1 bucket, and 0 will record
into correctly.  The fact that it's the "underflow" bucket for this histogram
won't cause issues.

[asanka, 2016/01/23 04:42:59]

Ah. Got it.

I actually switched to a sparse histogram since having a vector of buckets isn't
really necessary given how infrequent these downloads are, and also because
we'll only be recording this once per PPAPI initiated download.

   if (requestor.is_valid()) {
     scoped_refptr<SafeBrowsingService> service =
         g_browser_process->safe_browsing_service();
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&CheckWhitelistOnIOThread, service, requestor, file,
-                   callback));
+                   alternate_extensions, callback));
     return;
   }
 
-  CheckFieldTrialOnAnyThread(file, GURL(), callback);
+  CheckFieldTrialOnAnyThread(file, alternate_extensions, GURL(), callback);
 }
 
 }  // namespace safe_browsing