Commit 266f4967 authored by Takashi Sakamoto's avatar Takashi Sakamoto Committed by Chromium LUCI CQ

Fix heap-use-after-free at reading list deletion on Android.

Inside RmoveEntryByURLImpl, ReadingListWillRemoveEntry invokes ReadingListManagerImpl::RemoveBookmark. The method removes the node which own the URL. So when invoking entries_-erase(url), url is not available.

BookmarkBridge: :DeleteBookmark deletes a reading list item b using url when the bookmark type is BOOKMARK_TYPE_READING_LIST (i.e. node->url()). The deletion invokes ReadingListModelImpl::RemoveEntryByURLImpl with the URL (as const GURL&).
Change-Id: I69d6afba8ca8ea72985dd2317db9f6a3a92fefc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2616641
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarBartek Nowierski <bartekn@chromium.org>
Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842951}
parent a836c532
......@@ -757,7 +757,12 @@ void BookmarkBridge::DeleteBookmark(
if (partner_bookmarks_shim_->IsPartnerBookmark(node)) {
partner_bookmarks_shim_->RemoveBookmark(node);
} else if (type == BookmarkType::BOOKMARK_TYPE_READING_LIST) {
reading_list_manager_->Delete(node->url());
// Inside the Delete method, node will be destroyed and node->url will be
// also destroyed. This causes heap-use-after-free at
// ReadingListModelImpl::RemoveEntryByURLImpl. To avoid the
// heap-use-after-free, make a copy of node->url() and use it.
GURL url(node->url());
reading_list_manager_->Delete(url);
} else {
bookmark_model_->Remove(node);
}
......
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