Skip to content

Commit

Permalink
[M110] login: Fix UAF by not disposing storage partition
Browse files Browse the repository at this point in the history
> http://crrev.com/c/3691982's assumption that storage partitions of
> previous gaia loads could be released when a new gaia load starts.
> This assumption does not hold and there could be pending calls using
> the partition even though the gaia webview has navigated away. This
> causes UAF crashes. This CL stops the crash by stop disposing the
> storage partitions and could be merged to release branches to stop
> fire there.
>
> Note this regresses the memory leak in crbug/1308831.
>
> Bug: 1382971, 1308831
> Change-Id: I93764836ebbafd1f2c4b6906b8a1d2660e37cfc4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4140176
> Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Alexander Alekseev <alemate@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1090590}

(cherry picked from commit b1bce82)

Change-Id: Ia547943e5064a976c07f672da57a71c226c770c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4152810
Commit-Queue: Achuith Bhandarkar <achuith@chromium.org>
Auto-Submit: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#202}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Xiyuan Xia authored and Chromium LUCI CQ committed Jan 10, 2023
1 parent 80f7ab4 commit 6aaae27
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 21 deletions.
14 changes: 0 additions & 14 deletions chrome/browser/ash/login/webview_login_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1062,20 +1062,6 @@ IN_PROC_BROWSER_TEST_F(WebviewLoginTest, StoragePartitionHandling) {
// The StoragePartition which is not in use is supposed to have been cleared.
EXPECT_EQ("", GetAllCookies(signin_frame_partition_1));
EXPECT_NE("", GetAllCookies(signin_frame_partition_2));

// Trigger another gaia load.
test::OobeJS().ClickOnPath(kBackButton);
WaitForGaiaPageBackButtonUpdate();
ExpectIdentifierPage();

// `signin_frame_partition_1` is disposed and no longer accessible.
bool found_signin_frame_partition_1 = false;
browser_context->ForEachStoragePartition(
base::BindLambdaForTesting([&](content::StoragePartition* partition) {
if (partition == signin_frame_partition_1)
found_signin_frame_partition_1 = true;
}));
EXPECT_FALSE(found_signin_frame_partition_1);
}

enum class FrameUrlOrigin { kSameOrigin, kDifferentOrigin };
Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/ui/webui/ash/login/gaia_screen_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -705,13 +705,6 @@ void GaiaScreenHandler::HandleAuthExtensionLoaded() {
// used during the current sign-in attempt.
extension_provided_client_cert_usage_observer_ =
std::make_unique<LoginClientCertUsageObserver>();

// Clear old storage partitions after a new sign-in page is loaded. All
// reference to the old storage partitions should be cleared.
login::SigninPartitionManager* signin_partition_manager =
login::SigninPartitionManager::Factory::GetForBrowserContext(
Profile::FromWebUI(web_ui()));
signin_partition_manager->DisposeOldStoragePartitions();
}

void GaiaScreenHandler::HandleWebviewLoadAborted(int error_code) {
Expand Down

0 comments on commit 6aaae27

Please sign in to comment.