[Backport] CVE-2024-9955: Use after free in Web Authentication

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5900439:
Clear pending write socket data state before sending it

`EnclaveWebSocketClient` should not modify member variables after
calling `InternalWrite`.

Fixed: 370133761
Change-Id: Id29639790583d15bfa82e261ffcd40867a006dd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5900439
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Ken Buchanan <[email protected]>
Auto-Submit: Ken Buchanan <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1362071}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/597920
Reviewed-by: Michal Klocek <[email protected]>

Author: kenrb

chromium/device/fido/enclave/enclave_websocket_client.cc

@@ -5,6 +5,7 @@
 #include "device/fido/enclave/enclave_websocket_client.h"
 
 #include <limits>
+#include <utility>
 
 #include "components/device_event_log/device_event_log.h"
 #include "device/fido/fido_constants.h"
@@ -90,6 +91,7 @@ void EnclaveWebSocketClient::Write(base::span<const uint8_t> data) {
       data.size() > std::numeric_limits<uint32_t>::max()) {
     FIDO_LOG(ERROR) << "Invalid WebSocket write.";
     ClosePipe(SocketStatus::kError);
+    // `this` may have been deleted at this point.
     return;
   }
 
@@ -103,6 +105,7 @@ void EnclaveWebSocketClient::Write(base::span<const uint8_t> data) {
   }
 
   InternalWrite(data);
+  // `this` may have been deleted at this point.
 }
 
 void EnclaveWebSocketClient::Connect() {
@@ -144,6 +147,7 @@ void EnclaveWebSocketClient::InternalWrite(base::span<const uint8_t> data) {
   if (result != MOJO_RESULT_OK) {
     FIDO_LOG(ERROR) << "Failed to write to WebSocket.";
     ClosePipe(SocketStatus::kError);
+    // `this` may have been deleted at this point.
   }
 }
 
@@ -160,6 +164,7 @@ void EnclaveWebSocketClient::OnFailure(const std::string& message,
                   << net_error << ", " << response_code;
 
   ClosePipe(SocketStatus::kError);
+  // `this` may have been deleted at this point.
 }
 
 void EnclaveWebSocketClient::OnConnectionEstablished(
@@ -194,8 +199,9 @@ void EnclaveWebSocketClient::OnConnectionEstablished(
   state_ = State::kOpen;
 
   if (pending_write_data_) {
-    InternalWrite(*pending_write_data_);
-    pending_write_data_ = absl::nullopt;
+    auto write_data = std::exchange(pending_write_data_, std::nullopt).value();
+    InternalWrite(write_data);
+    // `this` may have been deleted at this point.
   }
 }
 
@@ -211,6 +217,7 @@ void EnclaveWebSocketClient::OnDataFrame(
   if (data_len == 0) {
     if (finish) {
       ProcessCompletedResponse();
+      // `this` may have been deleted at this point.
     }
     return;
   }
@@ -224,13 +231,15 @@ void EnclaveWebSocketClient::OnDataFrame(
     FIDO_LOG(ERROR) << "Invalid WebSocket frame (type: "
                     << static_cast<int>(type) << ", len: " << data_len << ")";
     ClosePipe(SocketStatus::kError);
+    // `this` may have been deleted at this point.
     return;
   }
 
   pending_read_data_.resize(new_size);
   pending_read_finished_ = finish;
   client_receiver_.Pause();
   ReadFromDataPipe(MOJO_RESULT_OK, mojo::HandleSignalsState());
+  // `this` may have been deleted at this point.
 }
 
 void EnclaveWebSocketClient::OnDropChannel(bool was_clean,
@@ -240,6 +249,7 @@ void EnclaveWebSocketClient::OnDropChannel(bool was_clean,
   CHECK(state_ == State::kOpen || state_ == State::kConnecting);
 
   ClosePipe(SocketStatus::kSocketClosed);
+  // `this` may have been deleted at this point.
 }
 
 void EnclaveWebSocketClient::OnClosingHandshake() {}
@@ -268,6 +278,7 @@ void EnclaveWebSocketClient::ReadFromDataPipe(MojoResult,
       client_receiver_.Resume();
       if (pending_read_finished_) {
         ProcessCompletedResponse();
+        // `this` may have been deleted at this point.
       }
     }
   } else if (result == MOJO_RESULT_SHOULD_WAIT) {
@@ -276,6 +287,7 @@ void EnclaveWebSocketClient::ReadFromDataPipe(MojoResult,
     FIDO_LOG(ERROR) << "Reading WebSocket frame failed: "
                     << static_cast<int>(result);
     ClosePipe(SocketStatus::kError);
+    // `this` may have been deleted at this point.
   }
 }
 
@@ -301,6 +313,7 @@ void EnclaveWebSocketClient::ClosePipe(SocketStatus status) {
 
 void EnclaveWebSocketClient::OnMojoPipeDisconnect() {
   ClosePipe(SocketStatus::kSocketClosed);
+  // `this` may have been deleted at this point.
 }
 
 }  // namespace device::enclave

chromium/device/fido/enclave/enclave_websocket_client.h

@@ -81,6 +81,10 @@ class EnclaveWebSocketClient : public network::mojom::WebSocketHandshakeClient,
   };
 
   void Connect();
+
+  // All of the methods below have the potential to invoke the response
+  // callback, which can destroy this object. No data members should be
+  // accessed after calling one.
   void InternalWrite(base::span<const uint8_t> data);
   void ReadFromDataPipe(MojoResult, const mojo::HandleSignalsState&);
   void ProcessCompletedResponse();