Commit 8787a575 authored by David Roger's avatar David Roger Committed by Chromium LUCI CQ

[profiles] Fix use-after-free in ProfileManager when deleting profiles

ProfileManager::CleanUpDeletedProfiles() passes a raw base::Value pointer
to ProfileCleanedUp() to be run later asynchronously.
This is incorrect, as this raw pointer is pointing somewhere in the
middle of a list in preferences. If the preferences change in the
meantime, the pointer is invalid when it is used.

The base::Value is passed by value rather than by unique_ptr, as this is
now the recommended pattern.

Fixed: 1164410
Change-Id: Ifeebe3fe9f49cc7f991c738b73b24bdb6b017168
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2618339Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841848}
parent d11e8342
...@@ -283,10 +283,10 @@ void NukeProfileFromDisk(const base::FilePath& profile_path) { ...@@ -283,10 +283,10 @@ void NukeProfileFromDisk(const base::FilePath& profile_path) {
} }
// Called after a deleted profile was checked and cleaned up. // Called after a deleted profile was checked and cleaned up.
void ProfileCleanedUp(const base::Value* profile_path_value) { void ProfileCleanedUp(base::Value profile_path_value) {
ListPrefUpdate deleted_profiles(g_browser_process->local_state(), ListPrefUpdate deleted_profiles(g_browser_process->local_state(),
prefs::kProfilesDeleted); prefs::kProfilesDeleted);
deleted_profiles->Remove(*profile_path_value, nullptr); deleted_profiles->Remove(profile_path_value, nullptr);
} }
#if BUILDFLAG(ENABLE_EXTENSIONS) #if BUILDFLAG(ENABLE_EXTENSIONS)
...@@ -1040,11 +1040,11 @@ void ProfileManager::CleanUpDeletedProfiles() { ...@@ -1040,11 +1040,11 @@ void ProfileManager::CleanUpDeletedProfiles() {
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}, base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
base::BindOnce(&NukeProfileFromDisk, *profile_path), base::BindOnce(&NukeProfileFromDisk, *profile_path),
base::BindOnce(&ProfileCleanedUp, &value)); base::BindOnce(&ProfileCleanedUp, value.Clone()));
} else { } else {
// Everything is fine, the profile was removed on shutdown. // Everything is fine, the profile was removed on shutdown.
content::GetUIThreadTaskRunner({})->PostTask( content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE, base::BindOnce(&ProfileCleanedUp, &value)); FROM_HERE, base::BindOnce(&ProfileCleanedUp, value.Clone()));
} }
} else { } else {
LOG(ERROR) << "Found invalid profile path in deleted_profiles: " LOG(ERROR) << "Found invalid profile path in deleted_profiles: "
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment