Commit 405c0146 authored by Ovidio Henriquez's avatar Ovidio Henriquez Committed by Commit Bot

Fix OOB in OnBluetoothScanningPromptEvent

This changes fixes an OOB access that may occur in
WebBluetoothServiceImpl::OnBluetoothScanningPromptEvent(). The method
assumes that |scanning_clients_| will be populated when the method is
called, however it can be cleared if a Mojo connection error is
triggered.

The method now returns if |scanning_clients_| is empty, and it uses the
back() and pop() methods of vector to further prevent accidental OOB
access. Additionally, in BluetoothDeviceScanningPromptController, the
EventHandler binding is updated so that the lifetime of the class is
associated with the binding.

Bug: 1024116
Change-Id: I2008f7bc1ce65be1d94d39370ac8593f5ff418e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1916686
Commit-Queue: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715472}
parent 1b24e32a
......@@ -31,7 +31,7 @@ void BluetoothDeviceScanningPromptController::ShowPermissionPrompt() {
BluetoothScanningPrompt::EventHandler prompt_event_handler =
base::BindRepeating(&BluetoothDeviceScanningPromptController::
OnBluetoothScanningPromptEvent,
base::Unretained(this));
weak_ptr_factory_.GetWeakPtr());
WebContentsDelegate* delegate =
WebContents::FromRenderFrameHost(render_frame_host_)->GetDelegate();
if (delegate) {
......
......@@ -295,16 +295,19 @@ bool WebBluetoothServiceImpl::IsDevicePaired(
void WebBluetoothServiceImpl::OnBluetoothScanningPromptEvent(
BluetoothScanningPrompt::Event event,
BluetoothDeviceScanningPromptController* prompt_controller) {
DCHECK(!scanning_clients_.empty());
// It is possible for |scanning_clients_| to be empty if a Mojo connection
// error has occurred before this method was called.
if (scanning_clients_.empty())
return;
auto client = scanning_clients_.end() - 1;
auto& client = scanning_clients_.back();
DCHECK((*client)->prompt_controller() == prompt_controller);
DCHECK(client->prompt_controller() == prompt_controller);
auto result = blink::mojom::WebBluetoothResult::SUCCESS;
if (event == BluetoothScanningPrompt::Event::kAllow) {
result = blink::mojom::WebBluetoothResult::SUCCESS;
StoreAllowedScanOptions((*client)->scan_options());
StoreAllowedScanOptions(client->scan_options());
} else if (event == BluetoothScanningPrompt::Event::kBlock) {
result = blink::mojom::WebBluetoothResult::SCANNING_BLOCKED;
const url::Origin requesting_origin =
......@@ -320,10 +323,10 @@ void WebBluetoothServiceImpl::OnBluetoothScanningPromptEvent(
NOTREACHED();
}
(*client)->RunRequestScanningStartCallback(std::move(result));
(*client)->set_prompt_controller(nullptr);
client->RunRequestScanningStartCallback(std::move(result));
client->set_prompt_controller(nullptr);
if (event == BluetoothScanningPrompt::Event::kAllow) {
(*client)->set_allow_send_event(true);
client->set_allow_send_event(true);
} else if (event == BluetoothScanningPrompt::Event::kBlock) {
// Here because user explicitly blocks the permission to do Bluetooth
// scanning in one request, it can be interpreted as user wants the current
......@@ -333,7 +336,7 @@ void WebBluetoothServiceImpl::OnBluetoothScanningPromptEvent(
allowed_scan_filters_.clear();
accept_all_advertisements_ = false;
} else if (event == BluetoothScanningPrompt::Event::kCanceled) {
scanning_clients_.erase(client);
scanning_clients_.pop_back();
} else {
NOTREACHED();
}
......@@ -1830,7 +1833,6 @@ void WebBluetoothServiceImpl::ClearState() {
receiver_.reset();
characteristic_id_to_notify_session_.clear();
scanning_clients_.clear();
pending_primary_services_requests_.clear();
descriptor_id_to_characteristic_id_.clear();
characteristic_id_to_service_id_.clear();
......@@ -1839,6 +1841,7 @@ void WebBluetoothServiceImpl::ClearState() {
new FrameConnectedBluetoothDevices(render_frame_host_));
device_chooser_controller_.reset();
device_scanning_prompt_controller_.reset();
scanning_clients_.clear();
allowed_scan_filters_.clear();
accept_all_advertisements_ = false;
BluetoothAdapterFactoryWrapper::Get().ReleaseAdapter(this);
......
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