mirror of
https://github.com/archlinuxarm/PKGBUILDs.git
synced 2024-11-08 22:45:43 +00:00
78 lines
3.5 KiB
Diff
78 lines
3.5 KiB
Diff
From 53478caee862624fc6d73516f8d64253854b146f Mon Sep 17 00:00:00 2001
|
|
From: Piotr Tworek <ptworek@vewd.com>
|
|
Date: Mon, 31 Aug 2020 21:03:58 +0000
|
|
Subject: [PATCH] Fix invalid "end" iterator usage in CookieMonster.
|
|
|
|
Commit 229623d76e8baf714c8569c9f4efc5de266cef8b has introduced the following
|
|
code in cookie_monster.cc.
|
|
|
|
// If this is the first cookie in |cookies_| with this key, increment the
|
|
// |num_keys_| counter.
|
|
bool different_prev =
|
|
inserted == cookies_.begin() || std::prev(inserted)->first != key;
|
|
bool different_next =
|
|
inserted == cookies_.end() || std::next(inserted)->first != key;
|
|
if (different_prev && different_next)
|
|
++num_keys_;
|
|
|
|
The "inserted" iterator is something that has been returned from
|
|
std::multimap::insert. At first glance it looks reasonable. The code
|
|
tries to determine if there are already similar elements with the same
|
|
key in the map. Unfortunately the expression calculating the value of
|
|
different_next can potentially use the end iterator to the map. The
|
|
"inserted == cookies_.end()" part of the expression will always evaluate
|
|
to false since the newly inserted element has to be in the map and
|
|
cookies_.end() points to the first element outside the map. If the
|
|
inserted happens to be the last element in the map the second part of
|
|
the expression will grab the end iterator by calling std::next(inserted)
|
|
and then will try to use it leading to invalid memory access.
|
|
|
|
Given the fact that cookies_ is a std::multimap we should not even need
|
|
to calculate the value of different_next. It should always be true.
|
|
|
|
"If the container has elements with equivalent key, inserts at the
|
|
upper bound of that range.(since C++11)"
|
|
|
|
See: https://en.cppreference.com/w/cpp/container/multimap/insert
|
|
|
|
Bug: 1120240
|
|
Change-Id: I8928c294ac4daf72349a2331b31b017c1d015da0
|
|
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368872
|
|
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
|
|
Commit-Queue: Piotr Tworek <ptworek@vewd.com>
|
|
Cr-Commit-Position: refs/heads/master@{#803260}
|
|
---
|
|
net/cookies/cookie_monster.cc | 13 +++++++++----
|
|
1 file changed, 9 insertions(+), 4 deletions(-)
|
|
|
|
diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
|
|
index 265deed0e52..140b61a81dc 100644
|
|
--- a/net/cookies/cookie_monster.cc
|
|
+++ b/net/cookies/cookie_monster.cc
|
|
@@ -1151,9 +1151,14 @@ CookieMonster::CookieMap::iterator CookieMonster::InternalInsertCookie(
|
|
// |num_keys_| counter.
|
|
bool different_prev =
|
|
inserted == cookies_.begin() || std::prev(inserted)->first != key;
|
|
- bool different_next =
|
|
- inserted == cookies_.end() || std::next(inserted)->first != key;
|
|
- if (different_prev && different_next)
|
|
+ // According to std::multiqueue documentation:
|
|
+ // "If the container has elements with equivalent key, inserts at the upper
|
|
+ // bound of that range. (since C++11)"
|
|
+ // This means that "inserted" iterator either points to the last element in
|
|
+ // the map, or the element succeeding it has to have different key.
|
|
+ DCHECK(std::next(inserted) == cookies_.end() ||
|
|
+ std::next(inserted)->first != key);
|
|
+ if (different_prev)
|
|
++num_keys_;
|
|
|
|
return inserted;
|
|
@@ -1381,7 +1386,7 @@ void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
|
|
bool different_prev =
|
|
it == cookies_.begin() || std::prev(it)->first != it->first;
|
|
bool different_next =
|
|
- it == cookies_.end() || std::next(it)->first != it->first;
|
|
+ std::next(it) == cookies_.end() || std::next(it)->first != it->first;
|
|
if (different_prev && different_next)
|
|
--num_keys_;
|
|
|