diff options
Diffstat (limited to 'fix-invalid-end-iterator-usage-in-CookieMonster.patch')
-rw-r--r-- | fix-invalid-end-iterator-usage-in-CookieMonster.patch | 78 |
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 000000000000..62abe8dc4123 --- /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_; + |