Ensure that buffers used by ImageDecoder haven't been neutered.

Since JavaScript may detach the underlying buffers, we need to check
to ensure they're still valid before using them for decoding.

Test: Updated unittests. Manual test case breaks.
Change-Id: Iefe5f8adf619cd6afdfedcb08a13c2996bfe0d32
Fixed: 1146761
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527542
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825615}

third_party/blink/renderer/modules/webcodecs/image_decoder_external.cc

@@ -120,9 +120,8 @@ ImageDecoderExternal::ImageDecoderExternal(ScriptState* script_state,
     return;
   }
 
-  // TODO(crbug.com/1073995): Data is owned by the caller who may be free to
-  // manipulate it. We will probably need to make a copy to our own internal
-  // data or neuter the buffers as seen by JS.
+  // Since data is owned by the caller who may be free to manipulate it, we must
+  // check HasValidEncodedData() before attempting to access |decoder_|.
   segment_reader_ = SegmentReader::CreateFromSkData(
       SkData::MakeWithoutCopy(buffer.Data(), buffer.ByteLengthAsSizeT()));
   if (!segment_reader_) {
@@ -266,6 +265,7 @@ void ImageDecoderExternal::Trace(Visitor* visitor) const {
 
 void ImageDecoderExternal::CreateImageDecoder() {
   DCHECK(!decoder_);
+  DCHECK(HasValidEncodedData());
 
   // TODO(crbug.com/1073995): We should probably call
   // ImageDecoder::SetMemoryAllocator() so that we can recycle frame buffers for
@@ -319,6 +319,13 @@ void ImageDecoderExternal::MaybeSatisfyPendingDecodes() {
       continue;
     }
 
+    if (!HasValidEncodedData()) {
+      request->exception = MakeGarbageCollected<DOMException>(
+          DOMExceptionCode::kInvalidStateError,
+          "Source data has been neutered");
+      continue;
+    }
+
     auto* image = decoder_->DecodeFrameBufferAtIndex(request->frame_index);
     if (decoder_->Failed() || !image) {
       // TODO(crbug.com/1073995): Include frameIndex in rejection?
@@ -397,6 +404,7 @@ void ImageDecoderExternal::MaybeSatisfyPendingDecodes() {
 }
 
 void ImageDecoderExternal::MaybeSatisfyPendingMetadataDecodes() {
+  DCHECK(HasValidEncodedData());
   DCHECK(decoder_);
   if (!decoder_->IsSizeAvailable() && !decoder_->Failed())
     return;
@@ -408,6 +416,9 @@ void ImageDecoderExternal::MaybeSatisfyPendingMetadataDecodes() {
 }
 
 void ImageDecoderExternal::MaybeUpdateMetadata() {
+  if (!HasValidEncodedData())
+    return;
+
   // Since we always create the decoder at construction, we need to wait until
   // at least the size is available before signaling that metadata has been
   // retrieved.
@@ -460,4 +471,22 @@ void ImageDecoderExternal::MaybeUpdateMetadata() {
   MaybeSatisfyPendingMetadataDecodes();
 }
 
+bool ImageDecoderExternal::HasValidEncodedData() const {
+  // If we keep an internal copy of the data, it's always valid.
+  if (stream_buffer_)
+    return true;
+
+  if (init_data_->data().IsArrayBuffer() &&
+      init_data_->data().GetAsArrayBuffer()->IsDetached()) {
+    return false;
+  }
+
+  if (init_data_->data().IsArrayBufferView() &&
+      !init_data_->data().GetAsArrayBufferView()->BaseAddress()) {
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/webcodecs/image_decoder_external.h

@@ -66,6 +66,10 @@ class MODULES_EXPORT ImageDecoderExternal final : public ScriptWrappable,
   void MaybeSatisfyPendingMetadataDecodes();
   void MaybeUpdateMetadata();
 
+  // Returns false if the decoder was constructed with an ArrayBuffer or
+  // ArrayBufferView that has since been neutered.
+  bool HasValidEncodedData() const;
+
   Member<ScriptState> script_state_;
 
   // Used when a ReadableStream is provided.

third_party/blink/renderer/modules/webcodecs/image_decoder_external_test.cc

@@ -98,6 +98,57 @@ TEST_F(ImageDecoderTest, DecodeEmpty) {
   EXPECT_TRUE(v8_scope.GetExceptionState().HadException());
 }
 
+TEST_F(ImageDecoderTest, DecodeNeuteredAtConstruction) {
+  V8TestingScope v8_scope;
+
+  auto* init = MakeGarbageCollected<ImageDecoderInit>();
+  auto* buffer = DOMArrayBuffer::Create(SharedBuffer::Create());
+
+  init->setType("image/png");
+  init->setData(
+      ArrayBufferOrArrayBufferViewOrReadableStream::FromArrayBuffer(buffer));
+
+  ArrayBufferContents contents;
+  ASSERT_TRUE(buffer->Transfer(v8_scope.GetIsolate(), contents));
+
+  auto* decoder = ImageDecoderExternal::Create(v8_scope.GetScriptState(), init,
+                                               v8_scope.GetExceptionState());
+  EXPECT_TRUE(decoder);
+  EXPECT_TRUE(v8_scope.GetExceptionState().HadException());
+}
+
+TEST_F(ImageDecoderTest, DecodeNeuteredAtDecodeTime) {
+  V8TestingScope v8_scope;
+
+  constexpr char kImageType[] = "image/gif";
+  EXPECT_TRUE(ImageDecoderExternal::canDecodeType(kImageType));
+
+  auto* init = MakeGarbageCollected<ImageDecoderInit>();
+  init->setType(kImageType);
+
+  constexpr char kTestFile[] = "images/resources/animated.gif";
+  auto data = ReadFile(kTestFile);
+  DCHECK(!data->IsEmpty()) << "Missing file: " << kTestFile;
+
+  auto* buffer = DOMArrayBuffer::Create(std::move(data));
+
+  init->setData(
+      ArrayBufferOrArrayBufferViewOrReadableStream::FromArrayBuffer(buffer));
+
+  auto* decoder = ImageDecoderExternal::Create(v8_scope.GetScriptState(), init,
+                                               v8_scope.GetExceptionState());
+  ASSERT_TRUE(decoder);
+  ASSERT_FALSE(v8_scope.GetExceptionState().HadException());
+
+  ArrayBufferContents contents;
+  ASSERT_TRUE(buffer->Transfer(v8_scope.GetIsolate(), contents));
+
+  auto promise = decoder->decode(0, true);
+  ScriptPromiseTester tester(v8_scope.GetScriptState(), promise);
+  tester.WaitUntilSettled();
+  ASSERT_TRUE(tester.IsRejected());
+}
+
 TEST_F(ImageDecoderTest, DecodeUnsupported) {
   V8TestingScope v8_scope;
   constexpr char kImageType[] = "image/svg+xml";