From 46be9fe1d78e9fa53a3bb4d1a401207e1a7ebb07 Mon Sep 17 00:00:00 2001
From: Thomas Guilbert <tguilbert@chromium.org>
Date: Mon, 31 Aug 2026 16:14:10 -0700
Subject: [PATCH] Fix UAF in openscreen_platform::NetUdpSocket

This CL adds weak pointer checks and routes client notifications through
a callback gate in NetUdpSocket to prevent a Use-After-Free if the
client's callbacks (e.g. OnRead, OnBound) synchronously destroy the
NetUdpSocket instance.

TAG=agy
CONV=3f403199-0d10-4bde-b609-19bcd1a13fd8

Bug: 553770012
Change-Id: I537bb98c9c0c33139a4c02a655e69d73f67bb171
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8309424
Auto-Submit: Jordan Bayles <jophba@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1689377}
---

diff --git a/components/openscreen_platform/net_udp_socket.cc b/components/openscreen_platform/net_udp_socket.cc
index d2d968e..9334890 100644
--- a/components/openscreen_platform/net_udp_socket.cc
+++ b/components/openscreen_platform/net_udp_socket.cc
@@ -47,17 +47,21 @@
 void NetUdpSocket::SendErrorToClient(openscreen::Error::Code openscreen_error,
                                      int net_error) {
   DVLOG(1) << __func__;
-  client_->OnError(
-      this, openscreen::Error(openscreen_error, net::ErrorToString(net_error)));
+  RunClientCallback([this, openscreen_error, net_error](Client& client) {
+    client.OnError(this, openscreen::Error(openscreen_error,
+                                           net::ErrorToString(net_error)));
+  });
 }
 
 void NetUdpSocket::DoRead() {
   DVLOG(3) << __func__;
-  while (HandleRecvFromResult(udp_socket_.RecvFrom(
-      read_buffer_.get(), openscreen::UdpPacket::kUdpMaxPacketSize,
-      &from_address_,
-      base::BindOnce(&NetUdpSocket::OnRecvFromCompleted,
-                     base::Unretained(this))))) {
+  base::WeakPtr<NetUdpSocket> weak_this = weak_ptr_factory_.GetWeakPtr();
+  while (weak_this &&
+         HandleRecvFromResult(udp_socket_.RecvFrom(
+             read_buffer_.get(), openscreen::UdpPacket::kUdpMaxPacketSize,
+             &from_address_,
+             base::BindOnce(&NetUdpSocket::OnRecvFromCompleted,
+                            weak_ptr_factory_.GetWeakPtr())))) {
   }
 }
 
@@ -69,9 +73,11 @@
   }
 
   if (result < 0) {
-    client_->OnRead(
-        this, openscreen::Error(openscreen::Error::Code::kSocketReadFailure,
-                                net::ErrorToString(result)));
+    RunClientCallback([this, result](Client& client) {
+      client.OnRead(
+          this, openscreen::Error(openscreen::Error::Code::kSocketReadFailure,
+                                  net::ErrorToString(result)));
+    });
     return false;
   }
 
@@ -80,8 +86,9 @@
   openscreen::UdpPacket packet(read_buffer_->data(),
                                UNSAFE_TODO(read_buffer_->data() + result));
   packet.set_source(openscreen_platform::ToOpenScreenEndPoint(from_address_));
-  client_->OnRead(this, std::move(packet));
-  return true;
+  return RunClientCallback([this, &packet](Client& client) {
+    client.OnRead(this, std::move(packet));
+  });
 }
 
 void NetUdpSocket::OnRecvFromCompleted(int result) {
@@ -95,9 +102,11 @@
   DVLOG(3) << __func__;
   send_pending_ = false;
   if (result < 0) {
-    client_->OnSendError(
-        this, openscreen::Error(openscreen::Error::Code::kSocketSendFailure,
-                                net::ErrorToString(result)));
+    RunClientCallback([this, result](Client& client) {
+      client.OnSendError(
+          this, openscreen::Error(openscreen::Error::Code::kSocketSendFailure,
+                                  net::ErrorToString(result)));
+    });
   }
 }
 
@@ -121,12 +130,9 @@
   net::IPEndPoint endpoint =
       openscreen_platform::ToNetEndPoint(local_endpoint_);
   int result = udp_socket_.Open(endpoint.GetFamily());
-  if (result != net::OK) {
-    SendErrorToClient(openscreen::Error::Code::kSocketBindFailure, result);
-    return;
+  if (result == net::OK) {
+    result = udp_socket_.Bind(endpoint);
   }
-
-  result = udp_socket_.Bind(endpoint);
   net::IPEndPoint local_endpoint;
   if (result == net::OK) {
     result = udp_socket_.GetLocalAddress(&local_endpoint);
@@ -138,8 +144,9 @@
   }
 
   local_endpoint_ = openscreen_platform::ToOpenScreenEndPoint(local_endpoint);
-  client_->OnBound(this);
-  DoRead();
+  if (RunClientCallback([this](Client& client) { client.OnBound(this); })) {
+    DoRead();
+  }
 }
 
 void NetUdpSocket::SetMulticastOutboundInterface(
@@ -170,17 +177,25 @@
   DVLOG(3) << __func__;
 
   if (send_pending_) {
-    client_->OnSendError(this,
+    RunClientCallback([this](Client& client) {
+      client.OnSendError(this,
                          openscreen::Error(openscreen::Error::Code::kAgain));
+    });
     return;
   }
 
   auto buffer = base::MakeRefCounted<net::IOBufferWithSize>(data.size());
   UNSAFE_TODO(memcpy(buffer->data(), data.data(), data.size()));
 
+  base::WeakPtr<NetUdpSocket> weak_this = weak_ptr_factory_.GetWeakPtr();
   const int result = udp_socket_.SendTo(
       buffer.get(), data.size(), openscreen_platform::ToNetEndPoint(dest),
-      base::BindOnce(&NetUdpSocket::OnSendToCompleted, base::Unretained(this)));
+      base::BindOnce(&NetUdpSocket::OnSendToCompleted,
+                     weak_ptr_factory_.GetWeakPtr()));
+  if (!weak_this) {
+    return;
+  }
+
   send_pending_ = true;
 
   if (result != net::ERR_IO_PENDING) {
diff --git a/components/openscreen_platform/net_udp_socket.h b/components/openscreen_platform/net_udp_socket.h
index 7af1f0ef..da3400ed 100644
--- a/components/openscreen_platform/net_udp_socket.h
+++ b/components/openscreen_platform/net_udp_socket.h
@@ -5,7 +5,10 @@
 #ifndef COMPONENTS_OPENSCREEN_PLATFORM_NET_UDP_SOCKET_H_
 #define COMPONENTS_OPENSCREEN_PLATFORM_NET_UDP_SOCKET_H_
 
+#include <utility>
+
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/socket/udp_socket.h"
@@ -26,10 +29,24 @@
   NetUdpSocket& operator=(NetUdpSocket&&) = delete;
 
  private:
+  // Dispatches a callback to `client_` and returns true if `*this` is still
+  // alive. Note: `client_` callbacks may synchronously destroy `this`.
+  template <typename Callback>
+  bool RunClientCallback(Callback&& callback) {
+    base::WeakPtr<NetUdpSocket> weak_this = weak_ptr_factory_.GetWeakPtr();
+    std::forward<Callback>(callback)(*client_);
+    return static_cast<bool>(weak_this);
+  }
+
   void SendErrorToClient(openscreen::Error::Code openscreen_error,
                          int net_error);
   void DoRead();
+
+  // Dispatches read data or error to `client_`. Returns true if more data
+  // should be read synchronously, or false if pending, on error, or if `this`
+  // was destroyed. Note: `client_` callbacks may synchronously destroy `this`.
   bool HandleRecvFromResult(int result);
+
   void OnRecvFromCompleted(int result);
   void OnSendToCompleted(int result);
 
@@ -55,6 +72,8 @@
   scoped_refptr<net::IOBuffer> read_buffer_;
   net::IPEndPoint from_address_;
   bool send_pending_ = false;
+
+  base::WeakPtrFactory<NetUdpSocket> weak_ptr_factory_{this};
 };