Fix UAF in WinRT event callback.

PiperOrigin-RevId: 949729415
This commit is contained in:
Francis Tsui
2026-07-17 13:21:00 -07:00
committed by Copybara-Service
parent 4301bc4fce
commit d1d15d266d
8 changed files with 78 additions and 88 deletions
+1 -1
View File
@@ -157,7 +157,7 @@ class BluetoothServerSocket final {
BluetoothServerSocket& operator=(const BluetoothServerSocket&) = default;
~BluetoothServerSocket() = default;
explicit BluetoothServerSocket(
std::unique_ptr<api::BluetoothServerSocket> socket)
std::shared_ptr<api::BluetoothServerSocket> socket)
: impl_(std::move(socket)) {}
// https://developer.android.com/reference/android/bluetooth/BluetoothServerSocket.html#accept()
@@ -19,7 +19,6 @@
#include <optional>
#include <string>
#include "absl/base/attributes.h"
#include "absl/functional/any_invocable.h"
#include "absl/strings/string_view.h"
#include "internal/platform/cancellation_flag.h"
@@ -29,8 +28,7 @@
#include "internal/platform/mac_address.h"
#include "internal/platform/output_stream.h"
namespace nearby {
namespace api {
namespace nearby::api {
// https://developer.android.com/reference/android/bluetooth/BluetoothDevice.html.
class BluetoothDevice {
@@ -273,7 +271,7 @@ class BluetoothClassicMedium {
// UUID.
//
// Returns nullptr error.
virtual std::unique_ptr<BluetoothServerSocket> ListenForService(
virtual std::shared_ptr<BluetoothServerSocket> ListenForService(
const std::string& service_name, const std::string& service_uuid) = 0;
// https://developer.android.com/reference/android/bluetooth/BluetoothDevice.html#createBond()
@@ -290,7 +288,6 @@ class BluetoothClassicMedium {
virtual void RemoveObserver(Observer* observer) = 0;
};
} // namespace api
} // namespace nearby
} // namespace nearby::api
#endif // PLATFORM_API_BLUETOOTH_CLASSIC_H_
@@ -240,10 +240,10 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
return socket;
}
std::unique_ptr<api::BluetoothServerSocket>
std::shared_ptr<api::BluetoothServerSocket>
BluetoothClassicMedium::ListenForService(const std::string& service_name,
const std::string& service_uuid) {
auto socket = std::make_unique<BluetoothServerSocket>(GetAdapter());
auto socket = std::make_shared<BluetoothServerSocket>(GetAdapter());
socket->SetCloseNotifier([this, uuid = service_uuid]() {
absl::MutexLock lock(mutex_);
sockets_.erase(uuid);
@@ -195,7 +195,7 @@ class BluetoothClassicMedium : public api::BluetoothClassicMedium {
// UUID.
//
// Returns nullptr on error.
std::unique_ptr<api::BluetoothServerSocket> ListenForService(
std::shared_ptr<api::BluetoothServerSocket> ListenForService(
const std::string& service_name, const std::string& service_uuid) override
ABSL_LOCKS_EXCLUDED(mutex_);
@@ -32,6 +32,7 @@
#include "internal/platform/implementation/bluetooth_classic.h"
#include "internal/platform/implementation/windows/bluetooth_adapter.h"
#include "internal/platform/implementation/windows/bluetooth_classic_device.h"
#include "internal/platform/implementation/windows/bluetooth_classic_server_socket.h"
#include "internal/platform/implementation/windows/bluetooth_classic_socket.h"
#include "internal/platform/implementation/windows/bluetooth_pairing.h"
#include "internal/platform/implementation/windows/generated/winrt/Windows.Devices.Bluetooth.Rfcomm.h"
@@ -43,8 +44,7 @@
#include "internal/platform/logging.h"
#include "internal/platform/mac_address.h"
namespace nearby {
namespace windows {
namespace nearby::windows {
namespace {
using ::winrt::Windows::Devices::Bluetooth::Rfcomm::RfcommDeviceService;
using ::winrt::Windows::Devices::Bluetooth::Rfcomm::RfcommServiceId;
@@ -124,7 +124,13 @@ BluetoothClassicMedium::BluetoothClassicMedium(
&BluetoothClassicMedium::OnScanModeChanged, this, std::placeholders::_1));
}
BluetoothClassicMedium::~BluetoothClassicMedium() {}
BluetoothClassicMedium::~BluetoothClassicMedium() {
// Clear the close notifier to prevent UAF if the server_socket_ outlives
// the BluetoothClassicMedium.
if (raw_server_socket_ != nullptr) {
raw_server_socket_->SetCloseNotifier(nullptr);
}
}
bool BluetoothClassicMedium::StartDiscovery(
BluetoothClassicMedium::DiscoveryCallback discovery_callback) {
@@ -259,7 +265,7 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
// UUID.
//
// Returns nullptr error.
std::unique_ptr<api::BluetoothServerSocket>
std::shared_ptr<api::BluetoothServerSocket>
BluetoothClassicMedium::ListenForService(const std::string& service_name,
const std::string& service_uuid) {
VLOG(1) << "ListenForService is called with service name: " << service_name
@@ -283,14 +289,24 @@ BluetoothClassicMedium::ListenForService(const std::string& service_name,
bool radio_discoverable =
scan_mode_ == BluetoothAdapter::ScanMode::kConnectableDiscoverable;
bool result = StartAdvertising(radio_discoverable);
if (rfcomm_provider_ != nullptr &&
is_radio_discoverable_ == radio_discoverable) {
LOG(WARNING) << __func__
<< ": Ignore StartAdvertising due to no change to "
"current advertising.";
return server_socket_;
}
if (!result) {
auto server_socket = StartAdvertising(radio_discoverable);
if (!server_socket) {
LOG(ERROR) << __func__ << ": Failed to start listening.";
return nullptr;
}
return std::move(server_socket_);
raw_server_socket_ = server_socket.get();
server_socket_ = std::move(server_socket);
return server_socket_;
}
api::BluetoothDevice* BluetoothClassicMedium::GetRemoteDevice(
@@ -833,25 +849,19 @@ bool BluetoothClassicMedium::IsWatcherRunning() {
(status == DeviceWatcherStatus::Stopping);
}
bool BluetoothClassicMedium::StartAdvertising(bool radio_discoverable) {
std::shared_ptr<BluetoothServerSocket>
BluetoothClassicMedium::StartAdvertising(bool radio_discoverable) {
LOG(INFO) << __func__
<< ": StartAdvertising is called with radio_discoverable: "
<< radio_discoverable << ".";
std::shared_ptr<BluetoothServerSocket> server_socket;
try {
if (rfcomm_provider_ != nullptr &&
is_radio_discoverable_ == radio_discoverable) {
LOG(WARNING) << __func__
<< ": Ignore StartAdvertising due to no change to "
"current advertising.";
return true;
}
if (rfcomm_provider_ != nullptr && !StopAdvertising()) {
LOG(WARNING) << __func__
<< ": Failed to StartAdvertising due to cannot stop "
"running advertising.";
return false;
return nullptr;
}
rfcomm_provider_ =
@@ -859,79 +869,57 @@ bool BluetoothClassicMedium::StartAdvertising(bool radio_discoverable) {
RfcommServiceId::FromUuid(winrt::guid(service_uuid_)))
.get();
server_socket_ = std::make_unique<BluetoothServerSocket>(
server_socket = BluetoothServerSocket::Create(
winrt::to_string(rfcomm_provider_.ServiceId().AsString()));
raw_server_socket_ = server_socket_.get();
if (!server_socket_->listen()) {
if (!server_socket->listen()) {
LOG(ERROR) << __func__
<< ": Failed to StartAdvertising due to cannot start socket.";
server_socket_->Close();
server_socket_ = nullptr;
rfcomm_provider_ = nullptr;
return false;
return nullptr;
}
server_socket_->SetCloseNotifier([&]() { StopAdvertising(); });
server_socket->SetCloseNotifier([&]() { StopAdvertising(); });
// Set the SDP attributes and start Bluetooth advertising
InitializeServiceSdpAttributes(rfcomm_provider_, service_name_);
// Start to advertising.
rfcomm_provider_.StartAdvertising(server_socket_->stream_socket_listener(),
rfcomm_provider_.StartAdvertising(server_socket->stream_socket_listener(),
radio_discoverable);
is_radio_discoverable_ = radio_discoverable;
LOG(INFO) << ": StartListening completed successfully.";
return true;
return server_socket;
} catch (std::exception exception) {
// We will log and eat the exception since the caller
// expects nullptr if it fails
LOG(ERROR) << __func__
<< ": Exception setting up for listen: " << exception.what();
if (server_socket_ != nullptr) {
server_socket_->Close();
server_socket_ = nullptr;
}
if (rfcomm_provider_ != nullptr) {
rfcomm_provider_ = nullptr;
}
return false;
return nullptr;
} catch (const winrt::hresult_error& ex) {
LOG(ERROR) << __func__ << ": Exception setting up for listen: " << ex.code()
<< ": " << winrt::to_string(ex.message());
if (server_socket_ != nullptr) {
server_socket_->Close();
server_socket_ = nullptr;
}
if (rfcomm_provider_ != nullptr) {
rfcomm_provider_ = nullptr;
}
return false;
return nullptr;
} catch (...) {
LOG(ERROR) << __func__ << ": Unknown exception.";
if (server_socket_ != nullptr) {
server_socket_->Close();
server_socket_ = nullptr;
}
if (rfcomm_provider_ != nullptr) {
rfcomm_provider_ = nullptr;
}
return false;
return nullptr;
}
}
bool BluetoothClassicMedium::StopAdvertising() {
VLOG(1) << __func__ << ": StopAdvertising is called";
bool result = false;
try {
if (rfcomm_provider_ == nullptr) {
LOG(ERROR) << __func__
@@ -940,12 +928,9 @@ bool BluetoothClassicMedium::StopAdvertising() {
}
rfcomm_provider_.StopAdvertising();
rfcomm_provider_ = nullptr;
raw_server_socket_ = nullptr;
server_socket_ = nullptr;
LOG(INFO) << ": StopAdvertising completed successfully.";
return true;
result = true;
} catch (std::exception exception) {
LOG(ERROR) << __func__
<< ": StopAdvertising exception: " << exception.what();
@@ -957,9 +942,9 @@ bool BluetoothClassicMedium::StopAdvertising() {
}
rfcomm_provider_ = nullptr;
raw_server_socket_ = nullptr;
server_socket_ = nullptr;
return false;
raw_server_socket_ = nullptr;
return result;
}
bool BluetoothClassicMedium::InitializeServiceSdpAttributes(
@@ -988,5 +973,4 @@ bool BluetoothClassicMedium::InitializeServiceSdpAttributes(
}
}
} // namespace windows
} // namespace nearby
} // namespace nearby::windows
@@ -34,8 +34,7 @@
#include "internal/platform/implementation/windows/generated/winrt/base.h"
#include "internal/platform/mac_address.h"
namespace nearby {
namespace windows {
namespace nearby::windows {
// Container of operations that can be performed over the Bluetooth Classic
// medium.
@@ -80,7 +79,7 @@ class BluetoothClassicMedium : public api::BluetoothClassicMedium {
// UUID.
//
// Returns nullptr error.
std::unique_ptr<api::BluetoothServerSocket> ListenForService(
std::shared_ptr<api::BluetoothServerSocket> ListenForService(
const std::string& service_name,
const std::string& service_uuid) override;
@@ -103,7 +102,8 @@ class BluetoothClassicMedium : public api::BluetoothClassicMedium {
private:
bool StartScanning();
bool StopScanning();
bool StartAdvertising(bool radio_discoverable);
std::shared_ptr<BluetoothServerSocket> StartAdvertising(
bool radio_discoverable);
bool StopAdvertising();
bool InitializeServiceSdpAttributes(
::winrt::Windows::Devices::Bluetooth::Rfcomm::RfcommServiceProvider
@@ -185,13 +185,15 @@ class BluetoothClassicMedium : public api::BluetoothClassicMedium {
// Used for advertising.
::winrt::Windows::Devices::Bluetooth::Rfcomm::RfcommServiceProvider
rfcomm_provider_ = nullptr;
std::unique_ptr<BluetoothServerSocket> server_socket_ = nullptr;
std::shared_ptr<api::BluetoothServerSocket> server_socket_;
// Raw pointer to the BluetoothServerSocket impl class that is held by the
// shared_ptr server_socket_. The lifetime of this pointer is guaranteed by
// the shared_ptr.
BluetoothServerSocket* raw_server_socket_ = nullptr;
bool is_radio_discoverable_ = false;
ObserverList<Observer> observers_;
};
} // namespace windows
} // namespace nearby
} // namespace nearby::windows
#endif // PLATFORM_IMPL_WINDOWS_BLUETOOTH_CLASSIC_MEDIUM_H_
@@ -26,10 +26,10 @@
#include "internal/platform/exception.h"
#include "internal/platform/implementation/bluetooth_classic.h"
#include "internal/platform/implementation/windows/bluetooth_classic_socket.h"
#include "internal/platform/implementation/windows/generated/winrt/Windows.Networking.Sockets.h"
#include "internal/platform/logging.h"
namespace nearby {
namespace windows {
namespace nearby::windows {
namespace {
using ::winrt::Windows::Networking::Sockets::SocketProtectionLevel;
using ::winrt::Windows::Networking::Sockets::SocketQualityOfService;
@@ -132,7 +132,8 @@ bool BluetoothServerSocket::listen() {
// Setup socket event of ConnectionReceived.
listener_event_token_ = stream_socket_listener_.ConnectionReceived(
{this, &BluetoothServerSocket::Listener_ConnectionReceived});
{shared_from_this(),
&BluetoothServerSocket::Listener_ConnectionReceived});
stream_socket_listener_
.BindServiceNameAsync(winrt::to_hstring(service_name_),
@@ -167,5 +168,4 @@ bool BluetoothServerSocket::listen() {
return ::winrt::fire_and_forget{};
}
} // namespace windows
} // namespace nearby
} // namespace nearby::windows
@@ -15,25 +15,30 @@
#ifndef PLATFORM_IMPL_WINDOWS_BLUETOOTH_CLASSIC_SERVER_SOCKET_H_
#define PLATFORM_IMPL_WINDOWS_BLUETOOTH_CLASSIC_SERVER_SOCKET_H_
#include <Windows.h>
#include <windows.h>
#include <memory>
#include <queue>
#include <string>
#include "absl/functional/any_invocable.h"
#include "absl/strings/string_view.h"
#include "absl/synchronization/mutex.h"
#include "internal/platform/exception.h"
#include "internal/platform/implementation/bluetooth_classic.h"
#include "internal/platform/implementation/windows/bluetooth_classic_socket.h"
#include "internal/platform/implementation/windows/generated/winrt/Windows.Networking.Sockets.h"
#include "internal/platform/implementation/windows/generated/winrt/base.h"
namespace nearby {
namespace windows {
namespace nearby::windows {
class BluetoothServerSocket : public api::BluetoothServerSocket {
class BluetoothServerSocket
: public api::BluetoothServerSocket,
public std::enable_shared_from_this<BluetoothServerSocket> {
public:
explicit BluetoothServerSocket(absl::string_view service_name);
static std::shared_ptr<BluetoothServerSocket> Create(
absl::string_view service_name) {
return std::shared_ptr<BluetoothServerSocket>(
new BluetoothServerSocket(service_name));
}
~BluetoothServerSocket() override;
@@ -65,6 +70,9 @@ class BluetoothServerSocket : public api::BluetoothServerSocket {
}
private:
// BluetoothServerSocket must be created as a shared_ptr.
explicit BluetoothServerSocket(absl::string_view service_name);
// The listener is accepting incoming connections
::winrt::fire_and_forget Listener_ConnectionReceived(
::winrt::Windows::Networking::Sockets::StreamSocketListener listener,
@@ -93,7 +101,6 @@ class BluetoothServerSocket : public api::BluetoothServerSocket {
bool closed_ = false;
};
} // namespace windows
} // namespace nearby
} // namespace nearby::windows
#endif // PLATFORM_IMPL_WINDOWS_BLUETOOTH_CLASSIC_SERVER_SOCKET_H_