Skip to content

Commit

Permalink
[FLEDGE] Only allow 20 active cross-origin IG joins and leaves per frame
Browse files Browse the repository at this point in the history
This is largely to prevent buggy pages from OOM-ing the browser process,
or from causing a ton of background requests long after a tab has been
closed.

This does not provide protection against compromised renderers
or malicious actors with lots of iframes from causing these issues,
but as neither is a security issue, best effort protection seems good
enough here, unless it becomes a problem.

Bug: 1315805
Change-Id: I43e1cae49d2b0a616d3c97e929fdb5c693028b0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3614951
Reviewed-by: Qingxin Wu <qingxinwu@google.com>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001567}
  • Loading branch information
Matt Menke authored and Chromium LUCI CQ committed May 10, 2022
1 parent 723deb1 commit 6ee98da
Show file tree
Hide file tree
Showing 9 changed files with 741 additions and 13 deletions.
450 changes: 450 additions & 0 deletions content/browser/interest_group/interest_group_browsertest.cc

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ namespace content {
// pending operations the renderer sends to the browser process at a time.
// * Add detailed error information to DevTools or as a Promise failure on
// rejection.
// * Add rate limiting in some manner in a way that avoids leaking information.
// * Figure out integration with IsInterestGroupAPIAllowed() - e.g., for
// cross-origin iframes, there are 3 origins (top-level frame, iframe,
// interest group frame). Currently we're not considering iframe origin at
Expand Down
4 changes: 4 additions & 0 deletions net/test/embedded_test_server/controllable_http_response.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ void ControllableHttpResponse::Done() {
state_ = State::DONE;
}

bool ControllableHttpResponse::has_received_request() {
return loop_.AnyQuitCalled();
}

void ControllableHttpResponse::OnRequest(
scoped_refptr<base::SingleThreadTaskRunner>
embedded_test_server_task_runner,
Expand Down
3 changes: 3 additions & 0 deletions net/test/embedded_test_server/controllable_http_response.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ class ControllableHttpResponse {
// Returns the HttpRequest after a call to WaitForRequest.
const HttpRequest* http_request() const { return http_request_.get(); }

// Returns whether or not the request has been received yet.
bool has_received_request();

private:
class Interceptor;

Expand Down
9 changes: 7 additions & 2 deletions third_party/blink/renderer/modules/ad_auction/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ blink_modules_sources("ad_auction") {
sources = [
"ads.cc",
"ads.h",
"join_leave_queue.h",
"navigator_auction.cc",
"navigator_auction.h",
"validate_blink_interest_group.cc",
Expand All @@ -19,12 +20,16 @@ blink_modules_sources("ad_auction") {

source_set("unit_tests") {
testonly = true
sources = [ "validate_blink_interest_group_test.cc" ]
sources = [
"join_leave_queue_test.cc",
"validate_blink_interest_group_test.cc",
]

deps = [
"//base",
"//mojo/public/cpp/test_support:test_utils",
"//testing/gtest:gtest",
"//testing/gmock",
"//testing/gtest",
"//third_party/blink/public:test_headers",
"//third_party/blink/public/common:headers",
"//third_party/blink/renderer/modules:modules",
Expand Down
81 changes: 81 additions & 0 deletions third_party/blink/renderer/modules/ad_auction/join_leave_queue.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright 2022 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.

#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_AD_AUCTION_JOIN_LEAVE_QUEUE_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_AD_AUCTION_JOIN_LEAVE_QUEUE_H_

#include "base/callback.h"
#include "base/check.h"
#include "third_party/blink/renderer/platform/wtf/deque.h"

namespace blink {

// A FIFO queue for interest group joins and leaves. It ensures there are never
// more than `max_active` started requests, adding requests to a queue if they
// can't be immediately started. Once the consumer informs it when a previously
// start request completes, it will start another request, if one is queued.
//
// This is a separate class so it can be unit tested.
template <typename T>
class JoinLeaveQueue {
public:
using StartCallback = base::RepeatingCallback<void(T&& operation)>;

// `max_active` is the maximum number of active operations at a time. `start`
// is invoked to start an operation. `this` may not called into or deleted
// while `start` is being invoked.
JoinLeaveQueue(int max_active, StartCallback start)
: max_active_(max_active), start_(start) {}

JoinLeaveQueue(JoinLeaveQueue&) = delete;
JoinLeaveQueue& operator=(JoinLeaveQueue&) = delete;

~JoinLeaveQueue() = default;

// If there are fewer than `max_active` operations, immediately invokes
// `start` with operation. Otherwise enqueues `operation`.
void Enqueue(T&& operation) {
if (num_active_ < max_active_) {
++num_active_;
start_.Run(std::move(operation));
return;
}

queue_.push_back(std::move(operation));
}

// Called when a previously started operation completes. Starts the next
// queued operation, if there is one.
void OnComplete() {
DCHECK_GT(num_active_, 0);

if (!queue_.empty()) {
DCHECK_EQ(num_active_, max_active_);
start_.Run(queue_.TakeFirst());
return;
}

--num_active_;
}

int num_active_for_testing() const { return num_active_; }

private:
// Maximum number of active operations.
const int max_active_;

// Callback to start the input operation.
const StartCallback start_;

// Current number of active operations. Active operations are not included in
// `queue_`.
int num_active_ = 0;

// FIFO queue of operations that have not yet started.
Deque<T> queue_;
};

} // namespace blink

#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_AD_AUCTION_JOIN_LEAVE_QUEUE_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2021 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 "third_party/blink/renderer/modules/ad_auction/join_leave_queue.h"

#include <memory>
#include <vector>

#include "base/bind.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace blink {

class JoinLeaveQueueTest : public testing::Test {
public:
JoinLeaveQueueTest()
: queue_(std::make_unique<JoinLeaveQueue<int>>(
/*max_active=*/2,
base::BindRepeating(&JoinLeaveQueueTest::Start,
base::Unretained(this)))) {}

protected:
void Start(int&& i) { start_order_.push_back(i); }

std::unique_ptr<JoinLeaveQueue<int>> queue_;

std::vector<int> start_order_;
};

TEST_F(JoinLeaveQueueTest, Basic) {
EXPECT_EQ(0, queue_->num_active_for_testing());

queue_->Enqueue(0);
EXPECT_THAT(start_order_, testing::ElementsAre(0));
EXPECT_EQ(1, queue_->num_active_for_testing());

queue_->Enqueue(1);
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1));
EXPECT_EQ(2, queue_->num_active_for_testing());

queue_->OnComplete();
queue_->OnComplete();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1));
EXPECT_EQ(0, queue_->num_active_for_testing());
}

TEST_F(JoinLeaveQueueTest, ExceedsLimit) {
queue_->Enqueue(0);
queue_->Enqueue(1);
queue_->Enqueue(2);
queue_->Enqueue(3);
queue_->Enqueue(4);
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1));
EXPECT_EQ(2, queue_->num_active_for_testing());

queue_->OnComplete();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1, 2));
EXPECT_EQ(2, queue_->num_active_for_testing());

queue_->OnComplete();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1, 2, 3));
EXPECT_EQ(2, queue_->num_active_for_testing());

queue_->OnComplete();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1, 2, 3, 4));
EXPECT_EQ(2, queue_->num_active_for_testing());

queue_->OnComplete();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1, 2, 3, 4));
EXPECT_EQ(1, queue_->num_active_for_testing());

queue_->OnComplete();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1, 2, 3, 4));
EXPECT_EQ(0, queue_->num_active_for_testing());
}

TEST_F(JoinLeaveQueueTest, DestroyedWithRequestsQueued) {
queue_->Enqueue(0);
queue_->Enqueue(1);
queue_->Enqueue(2);
queue_->Enqueue(3);

EXPECT_THAT(start_order_, testing::ElementsAre(0, 1));
EXPECT_EQ(2, queue_->num_active_for_testing());

queue_.reset();
EXPECT_THAT(start_order_, testing::ElementsAre(0, 1));
}

} // namespace blink
75 changes: 67 additions & 8 deletions third_party/blink/renderer/modules/ad_auction/navigator_auction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <utility>

#include "base/check.h"
#include "base/memory/scoped_refptr.h"
#include "base/time/time.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/common/features.h"
Expand Down Expand Up @@ -36,9 +37,11 @@
#include "third_party/blink/renderer/core/loader/document_loader.h"
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/modules/ad_auction/ads.h"
#include "third_party/blink/renderer/modules/ad_auction/join_leave_queue.h"
#include "third_party/blink/renderer/modules/ad_auction/validate_blink_interest_group.h"
#include "third_party/blink/renderer/modules/geolocation/geolocation_coordinates.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"
Expand All @@ -48,6 +51,12 @@ namespace blink {

namespace {

// The maximum number of active cross-site joins and leaves. Once these are hit,
// cross-site joins/leaves are queued until they drop below this number. Queued
// pending operations are dropped on destruction / navigation away.
const int kMaxActiveCrossSiteJoins = 20;
const int kMaxActiveCrossSiteLeaves = 20;

// Error string builders.

String ErrorInvalidInterestGroup(const AuctionAdInterestGroup& group,
Expand Down Expand Up @@ -868,6 +877,13 @@ void RecordCommonFledgeUseCounters(Document* document) {

NavigatorAuction::NavigatorAuction(Navigator& navigator)
: Supplement(navigator),
queued_cross_site_joins_(kMaxActiveCrossSiteJoins,
WTF::BindRepeating(&NavigatorAuction::StartJoin,
WrapWeakPersistent(this))),
queued_cross_site_leaves_(
kMaxActiveCrossSiteLeaves,
WTF::BindRepeating(&NavigatorAuction::StartLeave,
WrapWeakPersistent(this))),
ad_auction_service_(navigator.GetExecutionContext()) {
navigator.GetExecutionContext()->GetBrowserInterfaceBroker().GetInterface(
ad_auction_service_.BindNewPipeAndPassReceiver(
Expand Down Expand Up @@ -942,12 +958,23 @@ ScriptPromise NavigatorAuction::joinAdInterestGroup(
return ScriptPromise();
}

bool is_cross_origin =
!context->GetSecurityOrigin()->IsSameOriginWith(mojo_group->owner.get());

auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
ScriptPromise promise = resolver->Promise();
ad_auction_service_->JoinInterestGroup(
std::move(mojo_group),
mojom::blink::AdAuctionService::JoinInterestGroupCallback callback =
resolver->WrapCallbackInScriptScope(
WTF::Bind(&NavigatorAuction::JoinComplete, WrapPersistent(this))));
WTF::Bind(&NavigatorAuction::JoinComplete, WrapWeakPersistent(this),
is_cross_origin));

PendingJoin pending_join{std::move(mojo_group), std::move(callback)};
if (is_cross_origin) {
queued_cross_site_joins_.Enqueue(std::move(pending_join));
} else {
StartJoin(std::move(pending_join));
}

return promise;
}

Expand Down Expand Up @@ -992,12 +1019,25 @@ ScriptPromise NavigatorAuction::leaveAdInterestGroup(
return ScriptPromise();
}

bool is_cross_origin = !ExecutionContext::From(script_state)
->GetSecurityOrigin()
->IsSameOriginWith(owner.get());

auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
ScriptPromise promise = resolver->Promise();
ad_auction_service_->LeaveInterestGroup(
owner, group->name(),
mojom::blink::AdAuctionService::LeaveInterestGroupCallback callback =
resolver->WrapCallbackInScriptScope(
WTF::Bind(&NavigatorAuction::LeaveComplete, WrapPersistent(this))));
WTF::Bind(&NavigatorAuction::LeaveComplete, WrapWeakPersistent(this),
is_cross_origin));

PendingLeave pending_leave{std::move(owner), std::move(group->name()),
std::move(callback)};
if (is_cross_origin) {
queued_cross_site_leaves_.Enqueue(std::move(pending_leave));
} else {
StartLeave(std::move(pending_leave));
}

return promise;
}

Expand Down Expand Up @@ -1313,8 +1353,17 @@ void NavigatorAuction::FinalizeAdComplete(
}
}

void NavigatorAuction::JoinComplete(ScriptPromiseResolver* resolver,
void NavigatorAuction::StartJoin(PendingJoin&& pending_join) {
ad_auction_service_->JoinInterestGroup(std::move(pending_join.interest_group),
std::move(pending_join.callback));
}

void NavigatorAuction::JoinComplete(bool is_cross_origin,
ScriptPromiseResolver* resolver,
bool failed_well_known_check) {
if (is_cross_origin)
queued_cross_site_joins_.OnComplete();

if (failed_well_known_check) {
resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
resolver->GetScriptState()->GetIsolate(),
Expand All @@ -1325,8 +1374,18 @@ void NavigatorAuction::JoinComplete(ScriptPromiseResolver* resolver,
resolver->Resolve();
}

void NavigatorAuction::LeaveComplete(ScriptPromiseResolver* resolver,
void NavigatorAuction::StartLeave(PendingLeave&& pending_leave) {
ad_auction_service_->LeaveInterestGroup(pending_leave.owner,
pending_leave.name,
std::move(pending_leave.callback));
}

void NavigatorAuction::LeaveComplete(bool is_cross_origin,
ScriptPromiseResolver* resolver,
bool failed_well_known_check) {
if (is_cross_origin)
queued_cross_site_leaves_.OnComplete();

if (failed_well_known_check) {
resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
resolver->GetScriptState()->GetIsolate(),
Expand Down

0 comments on commit 6ee98da

Please sign in to comment.