Skip to content

GitLab

T
tangled
Commit 285cd875 authored by Dale Curtis's avatar Dale Curtis Committed by Chromium LUCI CQ
Browse files
Options

Don't allow WebCodecs fuzzers to fuzz if creation aborts. 

A review of similar Blink classes shows we should be returning nullptr
during the ::Create() method if there are exceptions. I've updated the
fuzzers to handle a null return value here now.

Fixed: 1167511, 1167532, 1167534
Change-Id: Idbbfaeac2e93eb1f4b8f02ad880724ac5454eac4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2638397
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844991}
parent d3f376a9
Hide whitespace changes
Inline Side-by-side
Showing with 143 additions and 130 deletions
third_party/blink/renderer/modules/webcodecs/audio_decoder.cc
View file @ 285cd875
...@@ -100,8 +100,9 @@ int AudioDecoderTraits::GetMaxDecodeRequests(const MediaDecoderType& decoder) { ...@@ -100,8 +100,9 @@ int AudioDecoderTraits::GetMaxDecodeRequests(const MediaDecoderType& decoder) {
AudioDecoder* AudioDecoder::Create(ScriptState* script_state, AudioDecoder* AudioDecoder::Create(ScriptState* script_state,
const AudioDecoderInit* init, const AudioDecoderInit* init,
ExceptionState& exception_state) { ExceptionState& exception_state) {
return MakeGarbageCollected<AudioDecoder>(script_state, init, auto* result =
exception_state); MakeGarbageCollected<AudioDecoder>(script_state, init, exception_state);
return exception_state.HadException() ? nullptr : result;
} }
// static // static
......
third_party/blink/renderer/modules/webcodecs/audio_decoder_fuzzer.cc
View file @ 285cd875
...@@ -70,36 +70,38 @@ DEFINE_TEXT_PROTO_FUZZER( ...@@ -70,36 +70,38 @@ DEFINE_TEXT_PROTO_FUZZER(
Persistent<AudioDecoder> audio_decoder = AudioDecoder::Create( Persistent<AudioDecoder> audio_decoder = AudioDecoder::Create(
script_state, audio_decoder_init, IGNORE_EXCEPTION_FOR_TESTING); script_state, audio_decoder_init, IGNORE_EXCEPTION_FOR_TESTING);
for (auto& invocation : proto.invocations()) { if (audio_decoder) {
switch (invocation.Api_case()) { for (auto& invocation : proto.invocations()) {
case wc_fuzzer::AudioDecoderApiInvocation::kConfigure: switch (invocation.Api_case()) {
audio_decoder->configure( case wc_fuzzer::AudioDecoderApiInvocation::kConfigure:
MakeAudioDecoderConfig(invocation.configure()), audio_decoder->configure(
IGNORE_EXCEPTION_FOR_TESTING); MakeAudioDecoderConfig(invocation.configure()),
break; IGNORE_EXCEPTION_FOR_TESTING);
case wc_fuzzer::AudioDecoderApiInvocation::kDecode: break;
audio_decoder->decode( case wc_fuzzer::AudioDecoderApiInvocation::kDecode:
MakeEncodedAudioChunk(invocation.decode().chunk()), audio_decoder->decode(
IGNORE_EXCEPTION_FOR_TESTING); MakeEncodedAudioChunk(invocation.decode().chunk()),
break; IGNORE_EXCEPTION_FOR_TESTING);
case wc_fuzzer::AudioDecoderApiInvocation::kFlush: { break;
// TODO(https://crbug.com/1119253): Fuzz whether to await resolution case wc_fuzzer::AudioDecoderApiInvocation::kFlush: {
// of the flush promise. // TODO(https://crbug.com/1119253): Fuzz whether to await resolution
audio_decoder->flush(IGNORE_EXCEPTION_FOR_TESTING); // of the flush promise.
break; audio_decoder->flush(IGNORE_EXCEPTION_FOR_TESTING);
break;
}
case wc_fuzzer::AudioDecoderApiInvocation::kReset:
audio_decoder->reset(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::AudioDecoderApiInvocation::kClose:
audio_decoder->close(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::AudioDecoderApiInvocation::API_NOT_SET:
break;
} }
case wc_fuzzer::AudioDecoderApiInvocation::kReset:
audio_decoder->reset(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::AudioDecoderApiInvocation::kClose:
audio_decoder->close(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::AudioDecoderApiInvocation::API_NOT_SET:
break;
}
// Give other tasks a chance to run (e.g. calling our output callback). // Give other tasks a chance to run (e.g. calling our output callback).
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
}
} }
} }
......
third_party/blink/renderer/modules/webcodecs/audio_encoder.cc
View file @ 285cd875
...@@ -23,8 +23,9 @@ const char* AudioEncoderTraits::GetNameForDevTools() { ...@@ -23,8 +23,9 @@ const char* AudioEncoderTraits::GetNameForDevTools() {
AudioEncoder* AudioEncoder::Create(ScriptState* script_state, AudioEncoder* AudioEncoder::Create(ScriptState* script_state,
const AudioEncoderInit* init, const AudioEncoderInit* init,
ExceptionState& exception_state) { ExceptionState& exception_state) {
return MakeGarbageCollected<AudioEncoder>(script_state, init, auto* result =
exception_state); MakeGarbageCollected<AudioEncoder>(script_state, init, exception_state);
return exception_state.HadException() ? nullptr : result;
} }
AudioEncoder::AudioEncoder(ScriptState* script_state, AudioEncoder::AudioEncoder(ScriptState* script_state,
......
third_party/blink/renderer/modules/webcodecs/image_decoder_external.cc
View file @ 285cd875
...@@ -33,8 +33,9 @@ ImageDecoderExternal* ImageDecoderExternal::Create( ...@@ -33,8 +33,9 @@ ImageDecoderExternal* ImageDecoderExternal::Create(
ScriptState* script_state, ScriptState* script_state,
const ImageDecoderInit* init, const ImageDecoderInit* init,
ExceptionState& exception_state) { ExceptionState& exception_state) {
return MakeGarbageCollected<ImageDecoderExternal>(script_state, init, auto* result = MakeGarbageCollected<ImageDecoderExternal>(script_state, init,
exception_state); exception_state);
return exception_state.HadException() ? nullptr : result;
} }
ImageDecoderExternal::DecodeRequest::DecodeRequest( ImageDecoderExternal::DecodeRequest::DecodeRequest(
......
third_party/blink/renderer/modules/webcodecs/image_decoder_external_test.cc
View file @ 285cd875
...@@ -89,7 +89,7 @@ TEST_F(ImageDecoderTest, DecodeEmpty) { ...@@ -89,7 +89,7 @@ TEST_F(ImageDecoderTest, DecodeEmpty) {
DOMArrayBuffer::Create(SharedBuffer::Create()))); DOMArrayBuffer::Create(SharedBuffer::Create())));
auto* decoder = ImageDecoderExternal::Create(v8_scope.GetScriptState(), init, auto* decoder = ImageDecoderExternal::Create(v8_scope.GetScriptState(), init,
v8_scope.GetExceptionState()); v8_scope.GetExceptionState());
EXPECT_TRUE(decoder); EXPECT_FALSE(decoder);
EXPECT_TRUE(v8_scope.GetExceptionState().HadException()); EXPECT_TRUE(v8_scope.GetExceptionState().HadException());
} }
...@@ -108,7 +108,7 @@ TEST_F(ImageDecoderTest, DecodeNeuteredAtConstruction) { ...@@ -108,7 +108,7 @@ TEST_F(ImageDecoderTest, DecodeNeuteredAtConstruction) {
auto* decoder = ImageDecoderExternal::Create(v8_scope.GetScriptState(), init, auto* decoder = ImageDecoderExternal::Create(v8_scope.GetScriptState(), init,
v8_scope.GetExceptionState()); v8_scope.GetExceptionState());
EXPECT_TRUE(decoder); EXPECT_FALSE(decoder);
EXPECT_TRUE(v8_scope.GetExceptionState().HadException()); EXPECT_TRUE(v8_scope.GetExceptionState().HadException());
} }
...@@ -150,7 +150,7 @@ TEST_F(ImageDecoderTest, DecodeUnsupported) { ...@@ -150,7 +150,7 @@ TEST_F(ImageDecoderTest, DecodeUnsupported) {
EXPECT_FALSE(ImageDecoderExternal::canDecodeType(kImageType)); EXPECT_FALSE(ImageDecoderExternal::canDecodeType(kImageType));
auto* decoder = auto* decoder =
CreateDecoder(&v8_scope, "images/resources/test.svg", kImageType); CreateDecoder(&v8_scope, "images/resources/test.svg", kImageType);
EXPECT_TRUE(decoder); EXPECT_FALSE(decoder);
EXPECT_TRUE(v8_scope.GetExceptionState().HadException()); EXPECT_TRUE(v8_scope.GetExceptionState().HadException());
} }
......
third_party/blink/renderer/modules/webcodecs/image_decoder_fuzzer.cc
View file @ 285cd875
...@@ -125,32 +125,34 @@ DEFINE_BINARY_PROTO_FUZZER( ...@@ -125,32 +125,34 @@ DEFINE_BINARY_PROTO_FUZZER(
ImageDecoderExternal::Create(script_state, image_decoder_init, ImageDecoderExternal::Create(script_state, image_decoder_init,
IGNORE_EXCEPTION_FOR_TESTING); IGNORE_EXCEPTION_FOR_TESTING);
// Promises will be fulfilled synchronously since we're using an array if (image_decoder) {
// buffer based source. // Promises will be fulfilled synchronously since we're using an array
for (auto& invocation : proto.invocations()) { // buffer based source.
switch (invocation.Api_case()) { for (auto& invocation : proto.invocations()) {
case wc_fuzzer::ImageDecoderApiInvocation::kDecodeImage: switch (invocation.Api_case()) {
image_decoder->decode( case wc_fuzzer::ImageDecoderApiInvocation::kDecodeImage:
invocation.decode_image().frame_index(), image_decoder->decode(
invocation.decode_image().complete_frames_only()); invocation.decode_image().frame_index(),
break; invocation.decode_image().complete_frames_only());
case wc_fuzzer::ImageDecoderApiInvocation::kDecodeMetadata: break;
image_decoder->decodeMetadata(); case wc_fuzzer::ImageDecoderApiInvocation::kDecodeMetadata:
break; image_decoder->decodeMetadata();
case wc_fuzzer::ImageDecoderApiInvocation::kSelectTrack: break;
image_decoder->selectTrack(invocation.select_track().track_id(), case wc_fuzzer::ImageDecoderApiInvocation::kSelectTrack:
IGNORE_EXCEPTION_FOR_TESTING); image_decoder->selectTrack(invocation.select_track().track_id(),
break; IGNORE_EXCEPTION_FOR_TESTING);
case wc_fuzzer::ImageDecoderApiInvocation::API_NOT_SET: break;
break; case wc_fuzzer::ImageDecoderApiInvocation::API_NOT_SET:
break;
}
// Give other tasks a chance to run (e.g. calling our output callback).
base::RunLoop().RunUntilIdle();
} }
// Give other tasks a chance to run (e.g. calling our output callback).
base::RunLoop().RunUntilIdle();
// TODO(crbug.com/1166925): Push the same image data incrementally into
// the fuzzer via a ReadableSource.
} }
// TODO(crbug.com/1166925): Push the same image data incrementally into
// the fuzzer via a ReadableSource.
} }
// Request a V8 GC. Oilpan will be invoked by the GC epilogue. // Request a V8 GC. Oilpan will be invoked by the GC epilogue.
......
third_party/blink/renderer/modules/webcodecs/video_decoder.cc
View file @ 285cd875
...@@ -213,8 +213,9 @@ int VideoDecoderTraits::GetMaxDecodeRequests(const MediaDecoderType& decoder) { ...@@ -213,8 +213,9 @@ int VideoDecoderTraits::GetMaxDecodeRequests(const MediaDecoderType& decoder) {
VideoDecoder* VideoDecoder::Create(ScriptState* script_state, VideoDecoder* VideoDecoder::Create(ScriptState* script_state,
const VideoDecoderInit* init, const VideoDecoderInit* init,
ExceptionState& exception_state) { ExceptionState& exception_state) {
return MakeGarbageCollected<VideoDecoder>(script_state, init, auto* result =
exception_state); MakeGarbageCollected<VideoDecoder>(script_state, init, exception_state);
return exception_state.HadException() ? nullptr : result;
} }
// static // static
......
third_party/blink/renderer/modules/webcodecs/video_decoder_fuzzer.cc
View file @ 285cd875
...@@ -70,36 +70,38 @@ DEFINE_TEXT_PROTO_FUZZER( ...@@ -70,36 +70,38 @@ DEFINE_TEXT_PROTO_FUZZER(
Persistent<VideoDecoder> video_decoder = VideoDecoder::Create( Persistent<VideoDecoder> video_decoder = VideoDecoder::Create(
script_state, video_decoder_init, IGNORE_EXCEPTION_FOR_TESTING); script_state, video_decoder_init, IGNORE_EXCEPTION_FOR_TESTING);
for (auto& invocation : proto.invocations()) { if (video_decoder) {
switch (invocation.Api_case()) { for (auto& invocation : proto.invocations()) {
case wc_fuzzer::VideoDecoderApiInvocation::kConfigure: switch (invocation.Api_case()) {
video_decoder->configure( case wc_fuzzer::VideoDecoderApiInvocation::kConfigure:
MakeVideoDecoderConfig(invocation.configure()), video_decoder->configure(
IGNORE_EXCEPTION_FOR_TESTING); MakeVideoDecoderConfig(invocation.configure()),
break; IGNORE_EXCEPTION_FOR_TESTING);
case wc_fuzzer::VideoDecoderApiInvocation::kDecode: break;
video_decoder->decode( case wc_fuzzer::VideoDecoderApiInvocation::kDecode:
MakeEncodedVideoChunk(invocation.decode().chunk()), video_decoder->decode(
IGNORE_EXCEPTION_FOR_TESTING); MakeEncodedVideoChunk(invocation.decode().chunk()),
break; IGNORE_EXCEPTION_FOR_TESTING);
case wc_fuzzer::VideoDecoderApiInvocation::kFlush: { break;
// TODO(https://crbug.com/1119253): Fuzz whether to await resolution case wc_fuzzer::VideoDecoderApiInvocation::kFlush: {
// of the flush promise. // TODO(https://crbug.com/1119253): Fuzz whether to await resolution
video_decoder->flush(IGNORE_EXCEPTION_FOR_TESTING); // of the flush promise.
break; video_decoder->flush(IGNORE_EXCEPTION_FOR_TESTING);
break;
}
case wc_fuzzer::VideoDecoderApiInvocation::kReset:
video_decoder->reset(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoDecoderApiInvocation::kClose:
video_decoder->close(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoDecoderApiInvocation::API_NOT_SET:
break;
} }
case wc_fuzzer::VideoDecoderApiInvocation::kReset:
video_decoder->reset(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoDecoderApiInvocation::kClose:
video_decoder->close(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoDecoderApiInvocation::API_NOT_SET:
break;
}
// Give other tasks a chance to run (e.g. calling our output callback). // Give other tasks a chance to run (e.g. calling our output callback).
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
}
} }
} }
......
third_party/blink/renderer/modules/webcodecs/video_encoder.cc
View file @ 285cd875
...@@ -185,8 +185,9 @@ const char* VideoEncoderTraits::GetNameForDevTools() { ...@@ -185,8 +185,9 @@ const char* VideoEncoderTraits::GetNameForDevTools() {
VideoEncoder* VideoEncoder::Create(ScriptState* script_state, VideoEncoder* VideoEncoder::Create(ScriptState* script_state,
const VideoEncoderInit* init, const VideoEncoderInit* init,
ExceptionState& exception_state) { ExceptionState& exception_state) {
return MakeGarbageCollected<VideoEncoder>(script_state, init, auto* result =
exception_state); MakeGarbageCollected<VideoEncoder>(script_state, init, exception_state);
return exception_state.HadException() ? nullptr : result;
} }
VideoEncoder::VideoEncoder(ScriptState* script_state, VideoEncoder::VideoEncoder(ScriptState* script_state,
......
third_party/blink/renderer/modules/webcodecs/video_encoder_fuzzer.cc
View file @ 285cd875
...@@ -77,45 +77,47 @@ DEFINE_TEXT_PROTO_FUZZER( ...@@ -77,45 +77,47 @@ DEFINE_TEXT_PROTO_FUZZER(
Persistent<VideoEncoder> video_encoder = VideoEncoder::Create( Persistent<VideoEncoder> video_encoder = VideoEncoder::Create(
script_state, video_encoder_init, IGNORE_EXCEPTION_FOR_TESTING); script_state, video_encoder_init, IGNORE_EXCEPTION_FOR_TESTING);
for (auto& invocation : proto.invocations()) { if (video_encoder) {
switch (invocation.Api_case()) { for (auto& invocation : proto.invocations()) {
case wc_fuzzer::VideoEncoderApiInvocation::kConfigure: switch (invocation.Api_case()) {
video_encoder->configure(MakeEncoderConfig(invocation.configure()), case wc_fuzzer::VideoEncoderApiInvocation::kConfigure:
IGNORE_EXCEPTION_FOR_TESTING); video_encoder->configure(MakeEncoderConfig(invocation.configure()),
break; IGNORE_EXCEPTION_FOR_TESTING);
case wc_fuzzer::VideoEncoderApiInvocation::kEncode: { break;
VideoFrame* frame = case wc_fuzzer::VideoEncoderApiInvocation::kEncode: {
MakeVideoFrame(script_state, invocation.encode().frame()); VideoFrame* frame =
// Often the fuzzer input will be too crazy to produce a valid frame MakeVideoFrame(script_state, invocation.encode().frame());
// (e.g. bitmap width > bitmap length). In these cases, return early // Often the fuzzer input will be too crazy to produce a valid frame
// to discourage this sort of fuzzer input. WebIDL doesn't allow // (e.g. bitmap width > bitmap length). In these cases, return early
// callers to pass null, so this is not a real concern. // to discourage this sort of fuzzer input. WebIDL doesn't allow
if (!frame) // callers to pass null, so this is not a real concern.
return; if (!frame)
return;
video_encoder->encode(
frame, MakeEncodeOptions(invocation.encode().options()), video_encoder->encode(
IGNORE_EXCEPTION_FOR_TESTING); frame, MakeEncodeOptions(invocation.encode().options()),
break; IGNORE_EXCEPTION_FOR_TESTING);
break;
}
case wc_fuzzer::VideoEncoderApiInvocation::kFlush: {
// TODO(https://crbug.com/1119253): Fuzz whether to await resolution
// of the flush promise.
video_encoder->flush(IGNORE_EXCEPTION_FOR_TESTING);
break;
}
case wc_fuzzer::VideoEncoderApiInvocation::kReset:
video_encoder->reset(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoEncoderApiInvocation::kClose:
video_encoder->close(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoEncoderApiInvocation::API_NOT_SET:
break;
} }
case wc_fuzzer::VideoEncoderApiInvocation::kFlush: {
// TODO(https://crbug.com/1119253): Fuzz whether to await resolution
// of the flush promise.
video_encoder->flush(IGNORE_EXCEPTION_FOR_TESTING);
break;
}
case wc_fuzzer::VideoEncoderApiInvocation::kReset:
video_encoder->reset(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoEncoderApiInvocation::kClose:
video_encoder->close(IGNORE_EXCEPTION_FOR_TESTING);
break;
case wc_fuzzer::VideoEncoderApiInvocation::API_NOT_SET:
break;
}
// Give other tasks a chance to run (e.g. calling our output callback). // Give other tasks a chance to run (e.g. calling our output callback).
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
}
} }
} }
......
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