Commit be3ad4d4 authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot

[Nearby] Fix UAF in FastInitiationManager

When FastInitiationManager calls the |error_callback_| from
StartAdvertising we destroy the instance in NearbySharingServiceImpl.
After that we can't access local fields anymore as they point to freed
memory.

Bug: b:154846208, 1109581
Change-Id: Ibdecd4c5759051b778ce833a0f70debd2b9b8e3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2320174
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791948}
parent ea229c44
...@@ -112,6 +112,7 @@ void FastInitiationManager::StopAdvertising( ...@@ -112,6 +112,7 @@ void FastInitiationManager::StopAdvertising(
if (!advertisement_) { if (!advertisement_) {
std::move(stop_callback_).Run(); std::move(stop_callback_).Run();
// |this| might be destroyed here, do not access local fields.
return; return;
} }
...@@ -181,8 +182,9 @@ void FastInitiationManager::OnRegisterAdvertisementError( ...@@ -181,8 +182,9 @@ void FastInitiationManager::OnRegisterAdvertisementError(
NS_LOG(ERROR) NS_LOG(ERROR)
<< "FastInitiationManager::StartAdvertising() failed with error code = " << "FastInitiationManager::StartAdvertising() failed with error code = "
<< error_code; << error_code;
std::move(start_error_callback_).Run();
start_callback_.Reset(); start_callback_.Reset();
std::move(start_error_callback_).Run();
// |this| might be destroyed here, do not access local fields.
} }
void FastInitiationManager::OnRestoreAdvertisingInterval() { void FastInitiationManager::OnRestoreAdvertisingInterval() {
...@@ -208,6 +210,7 @@ void FastInitiationManager::UnregisterAdvertisement() { ...@@ -208,6 +210,7 @@ void FastInitiationManager::UnregisterAdvertisement() {
void FastInitiationManager::OnUnregisterAdvertisement() { void FastInitiationManager::OnUnregisterAdvertisement() {
advertisement_.reset(); advertisement_.reset();
std::move(stop_callback_).Run(); std::move(stop_callback_).Run();
// |this| might be destroyed here, do not access local fields.
} }
void FastInitiationManager::OnUnregisterAdvertisementError( void FastInitiationManager::OnUnregisterAdvertisementError(
...@@ -217,6 +220,7 @@ void FastInitiationManager::OnUnregisterAdvertisementError( ...@@ -217,6 +220,7 @@ void FastInitiationManager::OnUnregisterAdvertisementError(
<< error_code; << error_code;
advertisement_.reset(); advertisement_.reset();
std::move(stop_callback_).Run(); std::move(stop_callback_).Run();
// |this| might be destroyed here, do not access local fields.
} }
std::vector<uint8_t> FastInitiationManager::GenerateFastInitV1Metadata( std::vector<uint8_t> FastInitiationManager::GenerateFastInitV1Metadata(
......
...@@ -8,7 +8,9 @@ ...@@ -8,7 +8,9 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/bind_helpers.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/test/bind_test_util.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "device/bluetooth/test/mock_bluetooth_adapter.h" #include "device/bluetooth/test/mock_bluetooth_adapter.h"
...@@ -268,6 +270,20 @@ TEST_F(NearbySharingFastInitiationManagerTest, TestStartAdvertising_Error) { ...@@ -268,6 +270,20 @@ TEST_F(NearbySharingFastInitiationManagerTest, TestStartAdvertising_Error) {
#endif #endif
} }
// Regression test for crbug.com/1109581.
TEST_F(NearbySharingFastInitiationManagerTest,
TestStartAdvertising_DeleteInErrorCallback) {
fast_initiation_manager_->StartAdvertising(
FastInitiationManager::FastInitType::kNotify, base::DoNothing(),
base::BindLambdaForTesting([&]() { fast_initiation_manager_.reset(); }));
std::move(register_args_->error_callback)
.Run(device::BluetoothAdvertisement::ErrorCode::
INVALID_ADVERTISEMENT_ERROR_CODE);
EXPECT_FALSE(fast_initiation_manager_);
}
TEST_F(NearbySharingFastInitiationManagerTest, TestStopAdvertising) { TEST_F(NearbySharingFastInitiationManagerTest, TestStopAdvertising) {
StartAdvertising(FastInitiationManager::FastInitType::kNotify); StartAdvertising(FastInitiationManager::FastInitType::kNotify);
auto fake_advertisement = base::MakeRefCounted<FakeBluetoothAdvertisement>(); auto fake_advertisement = base::MakeRefCounted<FakeBluetoothAdvertisement>();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment