Budget API calls should only succeed on secure origins

BUG=617971

Review-Url: https://codereview.chromium.org/2366533002
Cr-Commit-Position: refs/heads/master@{#420630}

chrome/browser/budget_service/budget_manager.cc

@@ -17,6 +17,7 @@
 #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"
 
@@ -68,12 +69,22 @@ double BudgetManager::GetCost(blink::mojom::BudgetOperationType type) {
 
 void BudgetManager::GetBudget(const url::Origin& origin,
                               const GetBudgetCallback& callback) {
+  if (origin.unique() || !content::IsOriginSecure(GURL(origin.Serialize()))) {
+    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) {
+  if (origin.unique() || !content::IsOriginSecure(GURL(origin.Serialize()))) {
+    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));
@@ -82,6 +93,11 @@ void BudgetManager::Reserve(const url::Origin& origin,
 void BudgetManager::Consume(const url::Origin& origin,
                             blink::mojom::BudgetOperationType type,
                             const ConsumeCallback& callback) {
+  if (origin.unique() || !content::IsOriginSecure(GURL(origin.Serialize()))) {
+    callback.Run(false /* success */);
+    return;
+  }
+
   bool found_reservation = false;
 
   // First, see if there is a reservation already.

chrome/browser/budget_service/budget_manager_unittest.cc

@@ -37,16 +37,18 @@ class BudgetManagerTest : public testing::Test {
 
   void SetSiteEngagementScore(double score) {
     SiteEngagementService* service = SiteEngagementService::Get(&profile_);
-    service->ResetScoreForURL(GURL(kTestOrigin), score);
+    service->ResetScoreForURL(GURL(origin().Serialize()), score);
   }
 
   Profile* profile() { return &profile_; }
   const url::Origin origin() const { return origin_; }
+  void SetOrigin(const url::Origin& origin) { origin_ = origin; }
 
   void ReserveCallback(base::Closure run_loop_closure,
                        blink::mojom::BudgetServiceErrorType error,
                        bool success) {
     success_ = (error == blink::mojom::BudgetServiceErrorType::NONE) && success;
+    error_ = error;
     run_loop_closure.Run();
   }
 
@@ -77,11 +79,12 @@ class BudgetManagerTest : public testing::Test {
 
   // Members for callbacks to set.
   bool success_;
+  blink::mojom::BudgetServiceErrorType error_;
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfile profile_;
-  const url::Origin origin_;
+  url::Origin origin_;
 };
 
 TEST_F(BudgetManagerTest, GetBudgetConsumedOverTime) {
@@ -107,3 +110,26 @@ TEST_F(BudgetManagerTest, GetBudgetConsumedOverTime) {
   // available.
   ASSERT_FALSE(ConsumeBudget(type));
 }
+
+TEST_F(BudgetManagerTest, TestInsecureOrigin) {
+  const blink::mojom::BudgetOperationType type =
+      blink::mojom::BudgetOperationType::SILENT_PUSH;
+  SetOrigin(url::Origin(GURL("http://example.com")));
+  SetSiteEngagementScore(kTestSES);
+
+  // Methods on the BudgetManager should only be allowed for secure origins.
+  ASSERT_FALSE(ReserveBudget(type));
+  ASSERT_EQ(blink::mojom::BudgetServiceErrorType::NOT_SUPPORTED, error_);
+  ASSERT_FALSE(ConsumeBudget(type));
+}
+
+TEST_F(BudgetManagerTest, TestUniqueOrigin) {
+  const blink::mojom::BudgetOperationType type =
+      blink::mojom::BudgetOperationType::SILENT_PUSH;
+  SetOrigin(url::Origin(GURL("file://example.com:443/etc/passwd")));
+
+  // Methods on the BudgetManager should not be allowed for unique origins.
+  ASSERT_FALSE(ReserveBudget(type));
+  ASSERT_EQ(blink::mojom::BudgetServiceErrorType::NOT_SUPPORTED, error_);
+  ASSERT_FALSE(ConsumeBudget(type));
+}

third_party/WebKit/Source/modules/budget/BudgetService.cpp

@@ -59,6 +59,10 @@ ScriptPromise BudgetService::getCost(ScriptState* scriptState, const AtomicStrin
 {
     DCHECK(m_service);
 
+    String errorMessage;
+    if (!scriptState->getExecutionContext()->isSecureContext(errorMessage))
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(SecurityError, errorMessage));
+
     mojom::blink::BudgetOperationType type = stringToOperationType(operation);
     if (type == mojom::blink::BudgetOperationType::INVALID_OPERATION)
         return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError, "Invalid operation type specified"));
@@ -80,12 +84,15 @@ ScriptPromise BudgetService::getBudget(ScriptState* scriptState)
 {
     DCHECK(m_service);
 
+    String errorMessage;
+    if (!scriptState->getExecutionContext()->isSecureContext(errorMessage))
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(SecurityError, errorMessage));
+
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     // Get the budget from the browser BudgetService.
     RefPtr<SecurityOrigin> origin(scriptState->getExecutionContext()->getSecurityOrigin());
-    // TODO(harkness): Check that this is a valid secure origin.
     m_service->GetBudget(origin, convertToBaseCallback(WTF::bind(&BudgetService::gotBudget, wrapPersistent(this), wrapPersistent(resolver))));
     return promise;
 }
@@ -113,12 +120,15 @@ ScriptPromise BudgetService::reserve(ScriptState* scriptState, const AtomicStrin
     if (type == mojom::blink::BudgetOperationType::INVALID_OPERATION)
         return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError, "Invalid operation type specified"));
 
+    String errorMessage;
+    if (!scriptState->getExecutionContext()->isSecureContext(errorMessage))
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(SecurityError, errorMessage));
+
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     // Call to the BudgetService to place the reservation.
     RefPtr<SecurityOrigin> origin(scriptState->getExecutionContext()->getSecurityOrigin());
-    // TODO(harkness): Check that this is a valid secure origin.
     m_service->Reserve(origin, type, convertToBaseCallback(WTF::bind(&BudgetService::gotReservation, wrapPersistent(this), wrapPersistent(resolver))));
     return promise;
 }