arc: Handle missing descriptor gracefully

This change updates the GATT read+write methods to gracefully handle the
scenario where the GATT descriptor is missing. This fixes a crash where
the descriptor is missing but the pointer is dereferenced without any
guards.

Bug: b/269646668
Test: Executed unit tests locally.
Change-Id: I0a474ea5b0745efd448487d06763b9f7fbef4c7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4291974
Commit-Queue: Chad Duffin <chadduffin@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1111889}

chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.cc

@@ -1667,7 +1667,18 @@ void ArcBluetoothBridge::ReadGattDescriptor(
   BluetoothRemoteGattDescriptor* descriptor =
       FindGattDescriptor(std::move(remote_addr), std::move(service_id),
                          std::move(char_id), std::move(desc_id));
-  DCHECK(descriptor);
+
+  if (!descriptor) {
+    BLUETOOTH_LOG(ERROR)
+        << __func__
+        << " failed to read GATT descriptor, descriptor does not exist.";
+    arc::mojom::BluetoothGattValuePtr gatt_value =
+        mojom::BluetoothGattValue::New();
+    gatt_value->status = mojom::BluetoothGattStatus::GATT_FAILURE;
+    std::move(callback).Run(std::move(gatt_value));
+    return;
+  }
+
   DCHECK(descriptor->GetPermissions() & kGattReadPermission);
 
   descriptor->ReadRemoteDescriptor(
@@ -1684,7 +1695,15 @@ void ArcBluetoothBridge::WriteGattDescriptor(
   BluetoothRemoteGattDescriptor* descriptor =
       FindGattDescriptor(std::move(remote_addr), std::move(service_id),
                          std::move(char_id), std::move(desc_id));
-  DCHECK(descriptor);
+
+  if (!descriptor) {
+    BLUETOOTH_LOG(ERROR)
+        << __func__
+        << " failed to write GATT descriptor, descriptor does not exist.";
+    std::move(callback).Run(mojom::BluetoothGattStatus::GATT_FAILURE);
+    return;
+  }
+
   DCHECK(descriptor->GetPermissions() & kGattWritePermission);
 
   if (value->value.empty()) {

chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge_unittest.cc

@@ -510,4 +510,48 @@ TEST_F(ArcBluetoothBridgeTest, ServiceChanged) {
   arc_bluetooth_bridge_->GattServiceChanged(adapter_.get(), service);
   EXPECT_TRUE(fake_bluetooth_instance_->get_service_changed_flag());
 }
+
+TEST_F(ArcBluetoothBridgeTest, ReadMissingDescriptorFailsGracefully) {
+  base::RunLoop run_loop;
+
+  // Pass clearly invalid values to guarantee that we won't be able to find a
+  // valid GATT descriptor.
+  arc_bluetooth_bridge_->ReadGattDescriptor(
+      /*remote_addr=*/mojom::BluetoothAddress::New(),
+      /*service_id=*/mojom::BluetoothGattServiceID::New(),
+      /*char_id=*/mojom::BluetoothGattID::New(),
+      /*desc_id=*/mojom::BluetoothGattID::New(),
+      base::BindOnce(
+          [](base::RepeatingClosure quit_closure,
+             mojom::BluetoothGattValuePtr value) {
+            ASSERT_TRUE(value);
+            EXPECT_TRUE(value->value.empty());
+            EXPECT_EQ(value->status, mojom::BluetoothGattStatus::GATT_FAILURE);
+            quit_closure.Run();
+          },
+          run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
+TEST_F(ArcBluetoothBridgeTest, WritingMissingDescriptorFailsGracefully) {
+  base::RunLoop run_loop;
+
+  // Pass clearly invalid values to guarantee that we won't be able to find a
+  // valid GATT descriptor.
+  arc_bluetooth_bridge_->WriteGattDescriptor(
+      /*remote_addr=*/mojom::BluetoothAddress::New(),
+      /*service_id=*/mojom::BluetoothGattServiceID::New(),
+      /*char_id=*/mojom::BluetoothGattID::New(),
+      /*desc_id=*/mojom::BluetoothGattID::New(),
+      /*value=*/mojom::BluetoothGattValue::New(),
+      base::BindOnce(
+          [](base::RepeatingClosure quit_closure,
+             mojom::BluetoothGattStatus status) {
+            EXPECT_EQ(status, mojom::BluetoothGattStatus::GATT_FAILURE);
+            quit_closure.Run();
+          },
+          run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
 }  // namespace arc