Ill in blink::TextDecoder::decode |
||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5650669494861824 Fuzzer: inferno_layout_test_unmodified Job Type: mac_asan_chrome Platform Id: mac Crash Type: Ill Crash Address: 0x000123abb2d1 Crash State: blink::TextDecoder::decode blink::TextDecoder::decode blink::V8TextDecoder::DecodeMethodCallback Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=605010:605037 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5650669494861824 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for instructions to reproduce this bug locally.
,
Jan 16
(6 days ago)
Automatically applying components based on crash stacktrace and information from OWNERS files. If this is incorrect, please apply the Test-Predator-Wrong-Components label.
,
Jan 16
(6 days ago)
Automatically adding ccs based on suspected regression changelists: [torque] use same mechanism as CSA_ASSERT for asserts and checks by tebbi@chromium.org - https://chromium.googlesource.com/v8/v8/+/b24e4a1be5d135eb5d701de6f74a88a537e25bdd MIPSR6: Fix compilation failure due to missing instruction patching by ivica.bogosavljevic@mips.com - https://chromium.googlesource.com/v8/v8/+/d8a958f58497311b13ca305f720337bfd45c10c6 If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.
,
Jan 16
(6 days ago)
My CL is probably innocent, it was supposed to make crashes from Torque easier to detect by Clusterfuzz. That being said, it seems it doesn't crash from Torque, which makes it quite mysterious how my CL could affect it.
,
Jan 17
(6 days ago)
,
Today
(15 hours ago)
This is an integer overflow:
==80431==ERROR: AddressSanitizer: ILL on unknown address 0x56288c97ca8b (pc 0x56288c97ca8b bp 0x7fc4c55031b0 sp 0x7fc4c55031b0 T37)
SCARINESS: 10 (signal)
#0 0x56288c97ca8a in unsigned int base::internal::CheckOnFailure::HandleFailure<unsigned int>() base/numerics/safe_conversions_impl.h:121:5
#1 0x56288c972eba in base::internal::StrictNumeric<unsigned int> base::internal::CheckedNumeric<unsigned int>::ValueOrDie<unsigned int, base::internal::CheckOnFailure>() const base/numerics/checked_math.h:88:18
#2 0x5628b8ac9b87 in WTF::TextCodecUTF8::Decode(char const*, unsigned int, WTF::FlushBehavior, bool, bool&) third_party/blink/renderer/platform/wtf/text/text_codec_utf8.cc:299:54
#3 0x5628c531adbe in blink::TextDecoder::decode(char const*, unsigned int, blink::TextDecodeOptions const*, blink::ExceptionState&) third_party/blink/renderer/modules/encoding/text_decoder.cc:113:22
#4 0x5628c531a46d in blink::TextDecoder::decode(blink::ArrayBufferOrArrayBufferView const&, blink::TextDecodeOptions const*, blink::ExceptionState&) third_party/blink/renderer/modules/encoding/text_decoder.cc:94:10
#5 0x5628c53171e5 in blink::text_decoder_v8_internal::DecodeMethod(v8::FunctionCallbackInfo<v8::Value> const&) gen/third_party/blink/renderer/bindings/modules/v8/v8_text_decoder.cc:131:25
#6 0x5628c5315160 in blink::V8TextDecoder::DecodeMethodCallback(v8::FunctionCallbackInfo<v8::Value> const&) gen/third_party/blink/renderer/bindings/modules/v8/v8_text_decoder.cc:214:3
#7 0x56289bbd93b6 in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) v8/src/api-arguments-inl.h:146:3
Also see reproducer:
[...]
o1135=new ArrayBuffer(4294967295);
o1188=o169.decode(o1135,{stream: true});
Michael, can you judge if is this something we need to handle or is it OK to just crash in that case?
,
Today
(15 hours ago)
jsbell, haraken: wdyt? ABs work on size_t and so decoding them as text should probably also rely on size_t and not uint32_t lengths.
,
Today
(12 hours ago)
See https://chromium.googlesource.com/chromium/src/+/dae5b388b44dae4dc11668dba210bbb92d72d969 and issue 901030 for context The issue is not the size of the AB, it's the size of the StringBuffer<LChar> needed to hold the decoded string, which can be bigger than the AB. StringImpl is limited to wtf_size_t so we can't decode to a string longer than that can hold. This is effectively an OOM - a JS call is asking to allocate something bigger than we can handle due to implementation limits. Not sure what the best/standard way to flag this is so ClusterFuzz stops pestering us about this case. haraken@, do you have guidance here? We could consider having the JS call throw rather than crashing here, although it would require significant plumbing through the text codec layers and probably some standards discussion. (I think we failed to make ArrayBuffer allocation throw rather than crash in similar cases?)
,
Today
(5 hours ago)
I agree that this is effectively an OOM. I think it's reasonable to mark this as WontFix. > Not sure what the best/standard way to flag this is so ClusterFuzz stops pestering us about this case. Do you know how ClusterFuzz is filtering out normal OOMs? I'm wondering if we could use that technology to filter out this case. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by ClusterFuzz
, Jan 16 (6 days ago)