summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTommy C. Li <tommycli@chromium.org>2023-02-22 11:09:24 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2023-02-27 09:13:17 +0000
commit447f5cb72905af87851b4c10c5e728f58cea46b5 (patch)
tree4f6a0170faa97cf5efc58d43d9e9598eb3e843f8
parent0cccf37c322f7ff7deeb0b24232f1d7336344450 (diff)
downloadqtwebengine-chromium-447f5cb72905af87851b4c10c5e728f58cea46b5.tar.gz
[Backport] Security bug 1414224
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4274984: Exclude Policy and Play API engines from Sync merging There's a security bug in which the call to ResetTemplateURLGUID can cause a policy-created engine to be deleted. This means that after the call, either the current `conflicting_turl` pointer, or future iterations in the loop may point to an already-freed TemplateURL, causing the use-after free bug. This CL addresses that by forbidding Policy-created and Play API engines from being merged into Synced engines. Although Play API engines aren't directly affected, they seem to also not be something that should be merged to Synced engines. (cherry picked from commit 315632458eb795ef9d9dce3fd1062f9e6f2c2077) Bug: 1414224 Change-Id: Ide43d71e9844e04a7ffe2e7ad2a522b6ca1535a3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4250623 Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1106249} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4274984 Commit-Queue: Krishna Govind <govind@chromium.org> Cr-Commit-Position: refs/branch-heads/5481@{#1238} Cr-Branched-From: 130f3e4d850f4bc7387cfb8d08aa993d288a67a9-refs/heads/main@{#1084008} (cherry picked from commit 06851790480e8e16a2913461d271437d525451a2) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/462770 Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r--chromium/components/search_engines/template_url_service.cc9
1 files changed, 8 insertions, 1 deletions
diff --git a/chromium/components/search_engines/template_url_service.cc b/chromium/components/search_engines/template_url_service.cc
index 4e22b44143b..c460e9acf42 100644
--- a/chromium/components/search_engines/template_url_service.cc
+++ b/chromium/components/search_engines/template_url_service.cc
@@ -2050,7 +2050,14 @@ void TemplateURLService::MergeInSyncTemplateURL(
keyword_to_turl_and_length_.equal_range(sync_turl->keyword());
for (auto it = match_range.first; it != match_range.second; ++it) {
TemplateURL* local_turl = it->second.first;
- if (local_turl->type() == TemplateURL::NORMAL) {
+ // The conflict resolution code below sometimes resets the TemplateURL's
+ // GUID, which can trigger deleting any Policy-created engines. Avoid this
+ // use-after-free bug by excluding any Policy-created engines. Also exclude
+ // Play API created engines, as those also seem local-only and should not
+ // be merged into Synced engines. crbug.com/1414224.
+ if (local_turl->type() == TemplateURL::NORMAL &&
+ !local_turl->created_by_policy() &&
+ !local_turl->created_from_play_api()) {
local_duplicates.push_back(local_turl);
}
}