Check for detached ArrayBuffers in Web Bluetooth writeValue functions

This change adds checks to verify that the ArrayBuffers passed to Web
Bluetooth's characteristic and descriptor writeValue() functions have
not been detached. If so then an InvalidStateError is thrown.

While the previous behavior was safe from use-after-frees it silently
set the characteristic or descriptor to an empty value which would be
difficult to debug.

Change-Id: Iee3d6b9f80d63961199b16a5de90bdd141ed152b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1604307Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658415}

third_party/blink/renderer/core/typed_arrays/dom_array_piece.cc

@@ -13,12 +13,11 @@ DOMArrayPiece::DOMArrayPiece(
     InitWithUnionOption option) {
   if (array_buffer_or_view.IsArrayBuffer()) {
     DOMArrayBuffer* array_buffer = array_buffer_or_view.GetAsArrayBuffer();
-    InitWithData(array_buffer->Data(), array_buffer->ByteLength());
+    InitWithArrayBuffer(array_buffer->Buffer());
   } else if (array_buffer_or_view.IsArrayBufferView()) {
     DOMArrayBufferView* array_buffer_view =
         array_buffer_or_view.GetAsArrayBufferView().View();
-    InitWithData(array_buffer_view->BaseAddress(),
-                 array_buffer_view->byteLength());
+    InitWithArrayBufferView(array_buffer_view->View());
   } else if (array_buffer_or_view.IsNull() &&
              option == kAllowNullPointToNullWithZeroSize) {
     InitWithData(nullptr, 0);

third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_characteristic.cc

@@ -176,6 +176,12 @@ ScriptPromise BluetoothRemoteGATTCharacteristic::writeValue(
         script_state, CreateInvalidCharacteristicError());
   }
 
+  if (value.IsNeutered()) {
+    return ScriptPromise::RejectWithDOMException(
+        script_state, DOMException::Create(DOMExceptionCode::kInvalidStateError,
+                                           "Value buffer has been detached."));
+  }
+
   // Partial implementation of writeValue algorithm:
   // https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattcharacteristic-writevalue
 

third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_descriptor.cc

@@ -118,6 +118,12 @@ ScriptPromise BluetoothRemoteGATTDescriptor::writeValue(
         script_state, CreateInvalidDescriptorError());
   }
 
+  if (value.IsNeutered()) {
+    return ScriptPromise::RejectWithDOMException(
+        script_state, DOMException::Create(DOMExceptionCode::kInvalidStateError,
+                                           "Value buffer has been detached."));
+  }
+
   // Partial implementation of writeValue algorithm:
   // https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattdescriptor-writevalue
 

third_party/blink/renderer/platform/wtf/typed_arrays/array_piece.cc

@@ -15,30 +15,22 @@ ArrayPiece::ArrayPiece() {
   InitNull();
 }
 
-ArrayPiece::ArrayPiece(void* data, unsigned byte_length) {
-  InitWithData(data, byte_length);
-}
-
 ArrayPiece::ArrayPiece(ArrayBuffer* buffer) {
-  if (buffer) {
-    InitWithData(buffer->Data(), SafeCast<unsigned>(buffer->ByteLength()));
-  } else {
-    InitNull();
-  }
+  InitWithArrayBuffer(buffer);
 }
 
 ArrayPiece::ArrayPiece(ArrayBufferView* buffer) {
-  if (buffer) {
-    InitWithData(buffer->BaseAddress(), buffer->ByteLength());
-  } else {
-    InitNull();
-  }
+  InitWithArrayBufferView(buffer);
 }
 
 bool ArrayPiece::IsNull() const {
   return is_null_;
 }
 
+bool ArrayPiece::IsNeutered() const {
+  return is_neutered_;
+}
+
 void* ArrayPiece::Data() const {
   DCHECK(!IsNull());
   return data_;
@@ -53,16 +45,36 @@ unsigned ArrayPiece::ByteLength() const {
   return byte_length_;
 }
 
+void ArrayPiece::InitWithArrayBuffer(ArrayBuffer* buffer) {
+  if (buffer) {
+    InitWithData(buffer->Data(), SafeCast<unsigned>(buffer->ByteLength()));
+    is_neutered_ = buffer->IsNeutered();
+  } else {
+    InitNull();
+  }
+}
+
+void ArrayPiece::InitWithArrayBufferView(ArrayBufferView* buffer) {
+  if (buffer) {
+    InitWithData(buffer->BaseAddress(), buffer->ByteLength());
+    is_neutered_ = buffer->Buffer() ? buffer->Buffer()->IsNeutered() : true;
+  } else {
+    InitNull();
+  }
+}
+
 void ArrayPiece::InitWithData(void* data, unsigned byte_length) {
   byte_length_ = byte_length;
   data_ = data;
   is_null_ = false;
+  is_neutered_ = false;
 }
 
 void ArrayPiece::InitNull() {
   byte_length_ = 0;
   data_ = nullptr;
   is_null_ = true;
+  is_neutered_ = false;
 }
 
 }  // namespace WTF

third_party/blink/renderer/platform/wtf/typed_arrays/array_piece.h

@@ -27,19 +27,20 @@ class WTF_EXPORT ArrayPiece {
   // Constructs a "null" ArrayPiece object.
   ArrayPiece();
 
-  ArrayPiece(void* data, unsigned byte_length);
-
   // Constructs an ArrayPiece from the given ArrayBuffer. If the input is a
   // nullptr, then the constructed instance will be isNull().
   ArrayPiece(ArrayBuffer*);
   ArrayPiece(ArrayBufferView*);
 
   bool IsNull() const;
+  bool IsNeutered() const;
   void* Data() const;
   unsigned char* Bytes() const;
   unsigned ByteLength() const;
 
  protected:
+  void InitWithArrayBuffer(ArrayBuffer*);
+  void InitWithArrayBufferView(ArrayBufferView*);
   void InitWithData(void* data, unsigned byte_length);
 
  private:
@@ -48,6 +49,7 @@ class WTF_EXPORT ArrayPiece {
   void* data_;
   unsigned byte_length_;
   bool is_null_;
+  bool is_neutered_;
 };
 
 }  // namespace WTF

third_party/blink/web_tests/external/wpt/bluetooth/characteristic/writeValue/buffer-is-detached.https.html

+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/bluetooth/resources/bluetooth-helpers.js"></script>
+<script>
+'use strict';
+
+function detachBuffer(buffer) {
+  window.postMessage('', '*', [buffer]);
+}
+
+bluetooth_test(async () => {
+  let {characteristic, fake_characteristic} =
+      await getMeasurementIntervalCharacteristic();
+
+  let typed_array = Uint8Array.of(1, 2);
+  detachBuffer(typed_array.buffer);
+  try {
+    await characteristic.writeValue(typed_array);
+    assert_unreached();
+  } catch (e) {
+    assert_equals(e.code, DOMException.INVALID_STATE_ERR, e.toString());
+  }
+
+  let array_buffer = Uint8Array.of(3, 4).buffer;
+  detachBuffer(array_buffer);
+  try {
+    await characteristic.writeValue(array_buffer);
+    assert_unreached();
+  } catch (e) {
+    assert_equals(e.code, DOMException.INVALID_STATE_ERR, e.toString());
+  }
+}, 'writeValue() fails when passed a detached buffer');
+</script>

third_party/blink/web_tests/external/wpt/bluetooth/descriptor/writeValue/buffer-is-detached.https.html

+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/bluetooth/resources/bluetooth-helpers.js"></script>
+<script>
+'use strict';
+
+function detachBuffer(buffer) {
+  window.postMessage('', '*', [buffer]);
+}
+
+bluetooth_test(async () => {
+  let {descriptor, fake_descriptor} =
+      await getUserDescriptionDescriptor();
+
+  let typed_array = Uint8Array.of(1, 2);
+  detachBuffer(typed_array.buffer);
+  try {
+    await descriptor.writeValue(typed_array);
+    assert_unreached();
+  } catch (e) {
+    assert_equals(e.code, DOMException.INVALID_STATE_ERR, e.toString());
+  }
+
+  let array_buffer = Uint8Array.of(3, 4).buffer;
+  detachBuffer(array_buffer);
+  try {
+    await descriptor.writeValue(array_buffer);
+    assert_unreached();
+  } catch (e) {
+    assert_equals(e.code, DOMException.INVALID_STATE_ERR, e.toString());
+  }
+}, 'writeValue() fails when passed a detached buffer');
+</script>