Commit 15a43eca authored by Ovidio Henriquez's avatar Ovidio Henriquez Committed by Commit Bot

Fix PermissionObserver OOB access crash

This change fixes a crash caused by an OOB access from the
|chooser_observer_| when removing the observer upon the destruction of
SiteSettingsHandler. The crash occurs when the following conditions are
met.

1. An off the record profile is created.
2. A chooser permission is granted in the off the record profile.
3. The chrome://settings/content page is opened in the main profile
   window.
4. The off the record window is closed.
5. The chrome://settings/content page is closed.

The SiteSettingsHandler will attempt to remove itself from the off the
record ChooserContextBase upon destruction, but the off the record
ChooserContextBase was already destroyed previously, therefore accessing
memory that has already been freed.

This change removes the logic that adds the SiteSettingsHandler to the
off the record ChooserContextBase, since the UI does not actually
display off the record chooser permissions anyways. I created
https://crbug.com/927372 for adding this feature.

Bug: 926501
Change-Id: I87359a3d67020a80b9fb698d5252c320436317de
Reviewed-on: https://chromium-review.googlesource.com/c/1447312Reviewed-by: default avatarDan Beam <dbeam@chromium.org>
Commit-Queue: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628468}
parent 2f701010
......@@ -1337,15 +1337,14 @@ void SiteSettingsHandler::ObserveSources() {
if (!profile_->HasOffTheRecordProfile())
return;
// At the moment, off the record chooser permissions are not included in the
// chooser permissions.
// TODO(https://crbug.com/927372): When chooser permissions are included,
// attach SiteSettingsHandler as an observer to the chooser contexts.
auto* map = HostContentSettingsMapFactory::GetForProfile(
profile_->GetOffTheRecordProfile());
if (!observer_.IsObserving(map))
observer_.Add(map);
auto* usb_context = UsbChooserContextFactory::GetForProfile(
profile_->GetOffTheRecordProfile());
if (!chooser_observer_.IsObserving(usb_context))
chooser_observer_.Add(usb_context);
}
void SiteSettingsHandler::TreeNodesAdded(ui::TreeModel* model,
......
......@@ -644,6 +644,8 @@ std::unique_ptr<base::ListValue> GetChooserExceptionListFromProfile(
const ChooserTypeNameEntry& chooser_type) {
auto exceptions = std::make_unique<base::ListValue>();
// TODO(https://crbug.com/927372): Combine the off the record permissions with
// the main profile permissions so that the UI is able to display them.
if (incognito) {
if (!profile->HasOffTheRecordProfile())
return exceptions;
......
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