[Backport] CVE-2021-21157: Use after free in Web Sockets

Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2655089: WebSocket: Don't clear event queue on destruction It's unnecessary to clear the event queue as it will be garbage collected anyway. Stop doing it. Also add a unit test for GC with pending events. This can only happen if the execution context changes while the events are pending. BUG=1170657 Change-Id: I01e5a687587f7471e88640c43f0dfe83e5c01bd1 Reviewed-by: Yutaka Hirano <[email protected]> Commit-Queue: Adam Rice <[email protected]> Cr-Commit-Position: refs/heads/master@{#848065} Reviewed-by: Allan Sandfeld Jensen <[email protected]>
qt · Feb 19, 2021 · ab1d490 · ab1d490
1 parent 4680e2d
commit ab1d490
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/chromium/third_party/blink/renderer/modules/websockets/dom_websocket.cc b/chromium/third_party/blink/renderer/modules/websockets/dom_websocket.cc
@@ -78,9 +78,7 @@ namespace blink {
 DOMWebSocket::EventQueue::EventQueue(EventTarget* target)
     : state_(kActive), target_(target) {}
 
-DOMWebSocket::EventQueue::~EventQueue() {
-  ContextDestroyed();
-}
+DOMWebSocket::EventQueue::~EventQueue() = default;
 
 void DOMWebSocket::EventQueue::Dispatch(Event* event) {
   switch (state_) {

diff --git a/chromium/third_party/blink/renderer/modules/websockets/dom_websocket_test.cc b/chromium/third_party/blink/renderer/modules/websockets/dom_websocket_test.cc
@@ -24,6 +24,7 @@
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/heap/impl/thread_state.h"
 #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
@@ -920,6 +921,32 @@ INSTANTIATE_TEST_SUITE_P(
     DOMWebSocketInvalidClosingCodeTest,
     testing::Values(0, 1, 998, 999, 1001, 2999, 5000, 9999, 65535));
 
+TEST(DOMWebSocketTest, GCWhileEventsPending) {
+  V8TestingScope scope;
+  {
+    DOMWebSocketTestScope websocket_scope(scope.GetExecutionContext());
+
+    EXPECT_CALL(websocket_scope.Channel(),
+                Connect(KURL("ws://example.com/"), String()))
+        .WillOnce(Return(true));
+    EXPECT_CALL(websocket_scope.Channel(), Disconnect());
+
+    auto& socket = websocket_scope.Socket();
+
+    // Cause events to be queued rather than fired.
+    socket.ContextLifecycleStateChanged(mojom::FrameLifecycleState::kPaused);
+
+    socket.Connect("ws://example.com/", Vector<String>(), ASSERT_NO_EXCEPTION);
+    socket.DidError();
+    socket.DidClose(DOMWebSocket::kClosingHandshakeIncomplete, 1006, "");
+
+    // Stop HasPendingActivity() from keeping the object alive.
+    socket.SetExecutionContext(nullptr);
+  }
+
+  ThreadState::Current()->CollectAllGarbageForTesting();
+}
+
 }  // namespace
 
 }  // namespace blink