summaryrefslogtreecommitdiffstats
path: root/fix-invalid-end-iterator-usage-in-CookieMonster.patch
diff options
context:
space:
mode:
Diffstat (limited to 'fix-invalid-end-iterator-usage-in-CookieMonster.patch')
-rw-r--r--fix-invalid-end-iterator-usage-in-CookieMonster.patch78
1 files changed, 78 insertions, 0 deletions
diff --git a/fix-invalid-end-iterator-usage-in-CookieMonster.patch b/fix-invalid-end-iterator-usage-in-CookieMonster.patch
new file mode 100644
index 0000000..62abe8d
--- /dev/null
+++ b/fix-invalid-end-iterator-usage-in-CookieMonster.patch
@@ -0,0 +1,78 @@
+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_;
+