New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 922529 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Ill in blink::TextDecoder::decode

Project Member Reported by ClusterFuzz, Jan 16 (6 days ago)

Issue description

Detailed 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.
 
Project Member

Comment 1 by ClusterFuzz, Jan 16 (6 days ago)

Labels: OS-Linux
Project Member

Comment 2 by ClusterFuzz, Jan 16 (6 days ago)

Components: Blink>TextEncoding
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 3 by ClusterFuzz, Jan 16 (6 days ago)

Cc: ivica.bo...@mips.com tebbi@chromium.org
Labels: Test-Predator-Auto-CC
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.

Comment 4 by tebbi@google.com, Jan 16 (6 days ago)

Labels: Test-Predator-Wrong-CLs
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.

Comment 5 by kkaluri@chromium.org, Jan 17 (6 days ago)

Components: Blink>JavaScript

Comment 6 by clemensh@chromium.org, Today (15 hours ago)

Cc: clemensh@chromium.org
Owner: mlippautz@chromium.org
Status: Assigned (was: Untriaged)
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?

Comment 7 by mlippautz@chromium.org, Today (15 hours ago)

Cc: mlippautz@chromium.org haraken@chromium.org
Owner: jsb...@chromium.org
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.

Comment 8 by jsb...@chromium.org, Today (12 hours ago)

Cc: e...@chromium.org
Owner: haraken@chromium.org
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?)

Comment 9 by haraken@chromium.org, 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