Skip to content

Commit

Permalink
[M115] Repro and fix out of bounds crash
Browse files Browse the repository at this point in the history
The invariant that size_limit_ never exceed bid_states_.size() was
broken when a bid failed due to a load failure (script fails to load),
leading to the bounds check in the bid_states_ vector crashing the
browser process during ApplySizeLimitAndSort().

***M115 note***:
I also cherry-pick one line from crrev.com/c/4570424 to fix
test failures with dangling pointers, since that CL landed after
the M115 branch.

(cherry picked from commit f738063)

Fixed: 1451572
Change-Id: I98d809d50f3aeb00840aabfba8bf925db9e05f2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4609062
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1156509}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4605465
Cr-Commit-Position: refs/branch-heads/5790@{#797}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
caraitto authored and Chromium LUCI CQ committed Jun 15, 2023
1 parent 1f2c05b commit b9c882e
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
1 change: 1 addition & 0 deletions content/browser/interest_group/interest_group_auction.cc
Expand Up @@ -1292,6 +1292,7 @@ class InterestGroupAuction::BuyerHelper
// to work.
std::swap(bid_states_[i], bid_states_.back());
bid_states_.pop_back();
size_limit_ = std::min(size_limit_, bid_states_.size());
continue;
}
DCHECK(bid_states_[i]->resume_generate_bid_callback);
Expand Down
54 changes: 54 additions & 0 deletions content/browser/interest_group/interest_group_browsertest.cc
Expand Up @@ -6932,6 +6932,60 @@ IN_PROC_BROWSER_TEST_F(InterestGroupBrowserTest,
}
}

// This test reproduces the crash reported in crbug.com/1451572.
IN_PROC_BROWSER_TEST_F(InterestGroupBrowserTest, RunAdAuctionRepro1451572) {
GURL test_url = https_server_->GetURL("a.test", "/page_with_iframe.html");
ASSERT_TRUE(NavigateToURL(shell(), test_url));
url::Origin test_origin = url::Origin::Create(test_url);
GURL ad_url = https_server_->GetURL("a.test", "/echo?render_cars");

const std::string repro_script = JsReplace(
R"(
(async function() {
// The crash occurs only if the second bid is filtered out before the first bid
// completes -- since the order this happens is arbitrary, the scenario is run
// several times to increase the chances of reproing the crash scenario.
//
// Two interest groups *are* required to repro the crash, and it's also required
// that only one of them has prioritization enabled and trusted signals set.

const ig_1= {
owner: $1,
name: "name_1",
biddingLogicUrl: $3,
ads: [{renderUrl: $1}],
};

const ig_2 = {
owner: $1,
name: "name_2",
// Intentionally use invalid bidding logic URL -- this results in the bid
// being filtered.
biddingLogicUrl: $1,
ads: [{renderUrl: $1}],
enableBiddingSignalsPrioritization: true,
trustedBiddingSignalsUrl: $1
};

const config = {
seller: $1,
decisionLogicUrl: $2,
interestGroupBuyers: [$1]
};

for(let i = 0; i < 20; i++) {
await navigator.joinAdInterestGroup(ig_1, 200);
await navigator.joinAdInterestGroup(ig_2, 200);
await navigator.runAdAuction(config);
}
})())",
test_origin,
https_server_->GetURL("a.test", "/interest_group/decision_logic.js"),
https_server_->GetURL("a.test", "/interest_group/bidding_logic.js"));

EXPECT_EQ(nullptr, EvalJs(shell(), repro_script));
}

// Test that the FLEDGE properly handles detached documents.
IN_PROC_BROWSER_TEST_F(InterestGroupFencedFrameBrowserTest,
DetachedDocumentDoesNotCrash) {
Expand Down
Expand Up @@ -168,7 +168,8 @@ class CONTENT_EXPORT TrustedSignalsRequestManager {

// If this request is currently assigned to a batched request, points to
// that request. nullptr otherwise.
raw_ptr<BatchedTrustedSignalsRequest> batched_request_ = nullptr;
raw_ptr<BatchedTrustedSignalsRequest, DanglingUntriaged> batched_request_ =
nullptr;
};

// Manages a single TrustedSignals object, which is associated with one or
Expand Down

0 comments on commit b9c882e

Please sign in to comment.