summaryrefslogtreecommitdiff
path: root/chromium
diff options
context:
space:
mode:
authorJosh Karlin <jkarlin@chromium.org>2021-03-02 03:00:18 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2021-04-19 22:35:11 +0000
commitaac48dd2cd1f34beb4d29a409c24f1ff4f98213c (patch)
tree80402c2b0bd3e025a7db17f0d6a346c52bbc3e3d /chromium
parentedc86cc74b9565c7d67341bbfa6efbe1859dbb8d (diff)
downloadqtwebengine-chromium-aac48dd2cd1f34beb4d29a409c24f1ff4f98213c.tar.gz
[Backport] CVE-2021-21214: Use after free in Network API
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2727306: Fix removal of observers in NetworkStateNotifier The NetworkStateNotifier has a per-thread list of observer pointers. If one is deleted mid-iteration, what we do is replace the pointer in the list with a 0, and add the index to the zeroed list of observers to remove after iteration completes. Well, the removal step was broken for cases where there were multiple elements to remove. It didn't adjust for the fact that the indexes shifted after each removal. Bug: 1170148 Change-Id: I446acaae5f8a805a58142848634a0ee8c5f90882 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#858853} Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium')
-rw-r--r--chromium/third_party/blink/renderer/platform/network/network_state_notifier.cc10
-rw-r--r--chromium/third_party/blink/renderer/platform/network/network_state_notifier_test.cc47
2 files changed, 55 insertions, 2 deletions
diff --git a/chromium/third_party/blink/renderer/platform/network/network_state_notifier.cc b/chromium/third_party/blink/renderer/platform/network/network_state_notifier.cc
index 32328e88187..c7227f63483 100644
--- a/chromium/third_party/blink/renderer/platform/network/network_state_notifier.cc
+++ b/chromium/third_party/blink/renderer/platform/network/network_state_notifier.cc
@@ -395,8 +395,14 @@ void NetworkStateNotifier::CollectZeroedObservers(
// If any observers were removed during the iteration they will have
// 0 values, clean them up.
- for (wtf_size_t i = 0; i < list->zeroed_observers.size(); ++i)
- list->observers.EraseAt(list->zeroed_observers[i]);
+ std::sort(list->zeroed_observers.begin(), list->zeroed_observers.end());
+ int removed = 0;
+ for (wtf_size_t i = 0; i < list->zeroed_observers.size(); ++i) {
+ int index_to_remove = list->zeroed_observers[i] - removed;
+ DCHECK_EQ(nullptr, list->observers[index_to_remove]);
+ list->observers.EraseAt(index_to_remove);
+ removed += 1;
+ }
list->zeroed_observers.clear();
diff --git a/chromium/third_party/blink/renderer/platform/network/network_state_notifier_test.cc b/chromium/third_party/blink/renderer/platform/network/network_state_notifier_test.cc
index eb2bd791529..f7c359235a8 100644
--- a/chromium/third_party/blink/renderer/platform/network/network_state_notifier_test.cc
+++ b/chromium/third_party/blink/renderer/platform/network/network_state_notifier_test.cc
@@ -528,6 +528,53 @@ TEST_F(NetworkStateNotifierTest, RemoveFutureObserverWhileNotifying) {
kUnknownThroughputMbps, SaveData::kOff));
}
+// It should be safe to remove multiple observers in one iteration.
+TEST_F(NetworkStateNotifierTest, RemoveMultipleObserversWhileNotifying) {
+ StateObserver observer1, observer2, observer3;
+ std::unique_ptr<NetworkStateNotifier::NetworkStateObserverHandle> handle1 =
+ notifier_.AddConnectionObserver(&observer1, GetTaskRunner());
+ std::unique_ptr<NetworkStateNotifier::NetworkStateObserverHandle> handle2 =
+ notifier_.AddConnectionObserver(&observer2, GetTaskRunner());
+ std::unique_ptr<NetworkStateNotifier::NetworkStateObserverHandle> handle3 =
+ notifier_.AddConnectionObserver(&observer3, GetTaskRunner());
+ observer1.RemoveObserverOnNotification(std::move(handle1));
+ observer3.RemoveObserverOnNotification(std::move(handle3));
+
+ // Running the first time should delete observers 1 and 3.
+ SetConnection(kWebConnectionTypeBluetooth, kBluetoothMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt,
+ kUnknownRtt, kUnknownThroughputMbps, SaveData::kOff);
+ EXPECT_TRUE(VerifyObservations(
+ observer1, kWebConnectionTypeBluetooth, kBluetoothMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt, kUnknownRtt,
+ kUnknownThroughputMbps, SaveData::kOff));
+ EXPECT_TRUE(VerifyObservations(
+ observer2, kWebConnectionTypeBluetooth, kBluetoothMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt, kUnknownRtt,
+ kUnknownThroughputMbps, SaveData::kOff));
+ EXPECT_TRUE(VerifyObservations(
+ observer3, kWebConnectionTypeBluetooth, kBluetoothMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt, kUnknownRtt,
+ kUnknownThroughputMbps, SaveData::kOff));
+
+ // Run again and only observer 2 should have been updated.
+ SetConnection(kWebConnectionTypeEthernet, kEthernetMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt,
+ kUnknownRtt, kUnknownThroughputMbps, SaveData::kOff);
+ EXPECT_TRUE(VerifyObservations(
+ observer1, kWebConnectionTypeBluetooth, kBluetoothMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt, kUnknownRtt,
+ kUnknownThroughputMbps, SaveData::kOff));
+ EXPECT_TRUE(VerifyObservations(
+ observer2, kWebConnectionTypeEthernet, kEthernetMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt, kUnknownRtt,
+ kUnknownThroughputMbps, SaveData::kOff));
+ EXPECT_TRUE(VerifyObservations(
+ observer3, kWebConnectionTypeBluetooth, kBluetoothMaxBandwidthMbps,
+ WebEffectiveConnectionType::kTypeUnknown, kUnknownRtt, kUnknownRtt,
+ kUnknownThroughputMbps, SaveData::kOff));
+}
+
TEST_F(NetworkStateNotifierTest, MultipleContextsAddObserver) {
StateObserver observer1, observer2;
std::unique_ptr<NetworkStateNotifier::NetworkStateObserverHandle> handle1 =