Budget API calls should only succeed on secure origins

chrome/browser/budget_service/budget_manager.cc

 // Copyright 2016 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/budget_service/budget_manager.h"
 
 #include <stdint.h>
 
 #include "base/callback.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/common/origin_util.h"
 #include "third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom.h"
 #include "url/origin.h"
 
 using content::BrowserThread;
 
 namespace {
 
 // Previously, budget information was stored in the prefs. If there is any old
 // information still there, clear it.
 // TODO(harkness): Remove once Chrome 56 has branched.
@@ skipping 31 matching lines @@
     case blink::mojom::BudgetOperationType::INVALID_OPERATION:
       return SiteEngagementScore::kMaxPoints + 1;
       // No default case.
   }
   NOTREACHED();
   return SiteEngagementScore::kMaxPoints + 1.0;
 }
 
 void BudgetManager::GetBudget(const url::Origin& origin,
                               const GetBudgetCallback& callback) {
+  const GURL url(origin.Serialize());
+  if (!url.SchemeIsHTTPOrHTTPS() || !content::IsOriginSecure(url)) {

[Peter Beverloo, 2016/09/22 14:27:29]

What's the added value of checking SchemeIsHTTPOrHTTPS()? I think we can just
rely on IsOriginSecure(). Elsewhere too.

[harkness, 2016/09/22 15:35:46]

By my reading of IsOriginSecure, it looks like it will allow file schemes, which
are not allowed by ServiceWorkers. 

I suppose it's really up to us what we want to allow in the spec, but my plan
was to mirror the service worker limits, and OriginCanAccessServiceWorkers has
botht he IsOriginSecure and the SchemeIsHTTPOrHTTPS check.

+    callback.Run(blink::mojom::BudgetServiceErrorType::NOT_SUPPORTED,
+                 mojo::Array<blink::mojom::BudgetStatePtr>());
+    return;
+  }
   db_.GetBudgetDetails(origin, callback);
 }
 
 void BudgetManager::Reserve(const url::Origin& origin,
                             blink::mojom::BudgetOperationType type,
                             const ReserveCallback& callback) {
+  const GURL url(origin.Serialize());
+  if (!url.SchemeIsHTTPOrHTTPS() || !content::IsOriginSecure(url)) {
+    callback.Run(blink::mojom::BudgetServiceErrorType::NOT_SUPPORTED,
+                 false /* success */);
+    return;
+  }
   db_.SpendBudget(origin, GetCost(type),
                   base::Bind(&BudgetManager::DidReserve,
                              weak_ptr_factory_.GetWeakPtr(), origin, callback));
 }
 
 void BudgetManager::Consume(const url::Origin& origin,
                             blink::mojom::BudgetOperationType type,
                             const ConsumeCallback& callback) {
+  const GURL url(origin.Serialize());
+  if (!url.SchemeIsHTTPOrHTTPS() || !content::IsOriginSecure(url)) {
+    callback.Run(false /* success */);
+    return;
+  }
+
   bool found_reservation = false;
 
   // First, see if there is a reservation already.
   auto count = reservation_map_.find(origin);
   if (count != reservation_map_.end()) {
     if (count->second == 1)
       reservation_map_.erase(origin);
     else
       reservation_map_[origin]--;
     found_reservation = true;
@@ skipping 26 matching lines @@
 void BudgetManager::DidReserve(const url::Origin& origin,
                                const ReserveCallback& callback,
                                blink::mojom::BudgetServiceErrorType error,
                                bool success) {
   // If the call succeeded, write the new reservation into the map.
   if (success && error == blink::mojom::BudgetServiceErrorType::NONE)
     reservation_map_[origin]++;
 
   callback.Run(error, success);
 }