From bf572c7831fd121701c5571371ccd5ff7514c42c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 15 Mar 2019 22:58:28 +0100 Subject: [PATCH] deps: V8: cherry-pick 91f0cd0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [ubsan] Fix various ClusterFuzz-found issues Fixing a few float and int overflows. Drive-by fix: with --experimental-wasm-bigint, Number values may not be used to initialize i64-typed globals. The existing code for doing that relied on UB; since it's a spec violation the fix is to throw instead. No regression test for 933103 because it will OOM anyway. No regression test for 932896 because it would be extremely slow. Bug: chromium:927894, chromium:927996, chromium:930086, chromium:932679, chromium:932896, chromium:933103, chromium:933134 Change-Id: Iae1c1ff1038af4512a52d3e56b8c4b75f2233314 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1495911 Commit-Queue: Jakob Kummerow Reviewed-by: Michael Starzinger Reviewed-by: Adam Klein Cr-Commit-Position: refs/heads/master@{#60075} Refs: https://github.com/v8/v8/commit/91f0cd00820a6e8d4567c1ce3a51d48a28165ab5 PR-URL: https://github.com/nodejs/node/pull/26685 Reviewed-By: Anna Henningsen Reviewed-By: Michaël Zasso Reviewed-By: Refael Ackermann --- common.gypi | 2 +- deps/v8/include/v8.h | 6 ++- deps/v8/src/builtins/builtins-string.cc | 7 +++- deps/v8/src/builtins/builtins-typed-array.cc | 19 ++++------ deps/v8/src/compiler/representation-change.cc | 9 +++-- deps/v8/src/objects/bigint.cc | 2 +- deps/v8/src/objects/fixed-array-inl.h | 3 ++ deps/v8/src/wasm/module-instantiate.cc | 26 +++++++++---- .../mjsunit/regress/wasm/regress-ubsan.js | 19 ++++++++++ deps/v8/test/mjsunit/ubsan-fuzzerbugs.js | 37 +++++++++++++++++++ 10 files changed, 105 insertions(+), 25 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/wasm/regress-ubsan.js diff --git a/common.gypi b/common.gypi index e2dc19aeee32c6..3b8328cd278d90 100644 --- a/common.gypi +++ b/common.gypi @@ -37,7 +37,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.2', + 'v8_embedder_string': '-node.3', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index 5e45cc762079d3..a5a88b685b85e6 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -10939,7 +10939,11 @@ int64_t Isolate::AdjustAmountOfExternalAllocatedMemory( reinterpret_cast(reinterpret_cast(this) + I::kExternalMemoryAtLastMarkCompactOffset); - const int64_t amount = *external_memory + change_in_bytes; + // Embedders are weird: we see both over- and underflows here. Perform the + // addition with unsigned types to avoid undefined behavior. + const int64_t amount = + static_cast(static_cast(change_in_bytes) + + static_cast(*external_memory)); *external_memory = amount; int64_t allocation_diff_since_last_mc = diff --git a/deps/v8/src/builtins/builtins-string.cc b/deps/v8/src/builtins/builtins-string.cc index d114a0e86bd851..43edd628d7b35b 100644 --- a/deps/v8/src/builtins/builtins-string.cc +++ b/deps/v8/src/builtins/builtins-string.cc @@ -448,7 +448,12 @@ BUILTIN(StringRaw) { Object::ToLength(isolate, raw_len)); IncrementalStringBuilder result_builder(isolate); - const uint32_t length = static_cast(raw_len->Number()); + // Intentional spec violation: we ignore {length} values >= 2^32, because + // assuming non-empty chunks they would generate too-long strings anyway. + const double raw_len_number = raw_len->Number(); + const uint32_t length = raw_len_number > std::numeric_limits::max() + ? std::numeric_limits::max() + : static_cast(raw_len_number); if (length > 0) { Handle first_element; ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, first_element, diff --git a/deps/v8/src/builtins/builtins-typed-array.cc b/deps/v8/src/builtins/builtins-typed-array.cc index 8c913c301deb41..ac1b23c8d3c27f 100644 --- a/deps/v8/src/builtins/builtins-typed-array.cc +++ b/deps/v8/src/builtins/builtins-typed-array.cc @@ -27,21 +27,18 @@ BUILTIN(TypedArrayPrototypeBuffer) { namespace { int64_t CapRelativeIndex(Handle num, int64_t minimum, int64_t maximum) { - int64_t relative; if (V8_LIKELY(num->IsSmi())) { - relative = Smi::ToInt(*num); + int64_t relative = Smi::ToInt(*num); + return relative < 0 ? std::max(relative + maximum, minimum) + : std::min(relative, maximum); } else { DCHECK(num->IsHeapNumber()); - double fp = HeapNumber::cast(*num)->value(); - if (V8_UNLIKELY(!std::isfinite(fp))) { - // +Infinity / -Infinity - DCHECK(!std::isnan(fp)); - return fp < 0 ? minimum : maximum; - } - relative = static_cast(fp); + double relative = HeapNumber::cast(*num)->value(); + DCHECK(!std::isnan(relative)); + return static_cast( + relative < 0 ? std::max(relative + maximum, minimum) + : std::min(relative, maximum)); } - return relative < 0 ? std::max(relative + maximum, minimum) - : std::min(relative, maximum); } } // namespace diff --git a/deps/v8/src/compiler/representation-change.cc b/deps/v8/src/compiler/representation-change.cc index 70b0d14e330fd2..5f7cd806564ed7 100644 --- a/deps/v8/src/compiler/representation-change.cc +++ b/deps/v8/src/compiler/representation-change.cc @@ -977,9 +977,12 @@ Node* RepresentationChanger::GetWord64RepresentationFor( break; case IrOpcode::kNumberConstant: { double const fv = OpParameter(node->op()); - int64_t const iv = static_cast(fv); - if (static_cast(iv) == fv) { - return jsgraph()->Int64Constant(iv); + using limits = std::numeric_limits; + if (fv <= limits::max() && fv >= limits::min()) { + int64_t const iv = static_cast(fv); + if (static_cast(iv) == fv) { + return jsgraph()->Int64Constant(iv); + } } break; } diff --git a/deps/v8/src/objects/bigint.cc b/deps/v8/src/objects/bigint.cc index 49a8728f15d0ca..fb69d5847a4dcb 100644 --- a/deps/v8/src/objects/bigint.cc +++ b/deps/v8/src/objects/bigint.cc @@ -1875,7 +1875,7 @@ MaybeHandle BigInt::AllocateFor( bits_min = (bits_min + roundup) >> kBitsPerCharTableShift; if (bits_min <= static_cast(kMaxInt)) { // Divide by kDigitsBits, rounding up. - int length = (static_cast(bits_min) + kDigitBits - 1) / kDigitBits; + int length = static_cast((bits_min + kDigitBits - 1) / kDigitBits); if (length <= kMaxLength) { Handle result = MutableBigInt::New(isolate, length, pretenure).ToHandleChecked(); diff --git a/deps/v8/src/objects/fixed-array-inl.h b/deps/v8/src/objects/fixed-array-inl.h index ad0684de1ba3e7..1614b6a53627c7 100644 --- a/deps/v8/src/objects/fixed-array-inl.h +++ b/deps/v8/src/objects/fixed-array-inl.h @@ -737,6 +737,9 @@ inline uint64_t FixedTypedArray::from(double value) { template <> inline float FixedTypedArray::from(double value) { + using limits = std::numeric_limits; + if (value > limits::max()) return limits::infinity(); + if (value < limits::lowest()) return -limits::infinity(); return static_cast(value); } diff --git a/deps/v8/src/wasm/module-instantiate.cc b/deps/v8/src/wasm/module-instantiate.cc index 237f047db8e986..d6a9bc3dfaa69c 100644 --- a/deps/v8/src/wasm/module-instantiate.cc +++ b/deps/v8/src/wasm/module-instantiate.cc @@ -3,7 +3,9 @@ // found in the LICENSE file. #include "src/wasm/module-instantiate.h" + #include "src/asmjs/asm-js.h" +#include "src/conversions-inl.h" #include "src/heap/heap-inl.h" // For CodeSpaceMemoryModificationScope. #include "src/property-descriptor.h" #include "src/utils.h" @@ -132,6 +134,7 @@ class InstanceBuilder { void LoadDataSegments(Handle instance); void WriteGlobalValue(const WasmGlobal& global, double value); + void WriteGlobalValue(const WasmGlobal& global, int64_t num); void WriteGlobalValue(const WasmGlobal& global, Handle value); @@ -653,25 +656,34 @@ void InstanceBuilder::WriteGlobalValue(const WasmGlobal& global, double num) { switch (global.type) { case kWasmI32: WriteLittleEndianValue(GetRawGlobalPtr(global), - static_cast(num)); + DoubleToInt32(num)); break; case kWasmI64: - WriteLittleEndianValue(GetRawGlobalPtr(global), - static_cast(num)); + // The Wasm-BigInt proposal currently says that i64 globals may + // only be initialized with BigInts. See: + // https://github.com/WebAssembly/JS-BigInt-integration/issues/12 + UNREACHABLE(); break; case kWasmF32: WriteLittleEndianValue(GetRawGlobalPtr(global), - static_cast(num)); + DoubleToFloat32(num)); break; case kWasmF64: - WriteLittleEndianValue(GetRawGlobalPtr(global), - static_cast(num)); + WriteLittleEndianValue(GetRawGlobalPtr(global), num); break; default: UNREACHABLE(); } } +void InstanceBuilder::WriteGlobalValue(const WasmGlobal& global, int64_t num) { + TRACE("init [globals_start=%p + %u] = %" PRId64 ", type = %s\n", + reinterpret_cast(raw_buffer_ptr(untagged_globals_, 0)), + global.offset, num, ValueTypes::TypeName(global.type)); + DCHECK_EQ(kWasmI64, global.type); + WriteLittleEndianValue(GetRawGlobalPtr(global), num); +} + void InstanceBuilder::WriteGlobalValue(const WasmGlobal& global, Handle value) { TRACE("init [globals_start=%p + %u] = ", @@ -1051,7 +1063,7 @@ bool InstanceBuilder::ProcessImportedGlobal(Handle instance, return true; } - if (value->IsNumber()) { + if (value->IsNumber() && global.type != kWasmI64) { WriteGlobalValue(global, value->Number()); return true; } diff --git a/deps/v8/test/mjsunit/regress/wasm/regress-ubsan.js b/deps/v8/test/mjsunit/regress/wasm/regress-ubsan.js new file mode 100644 index 00000000000000..530877580f6c64 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/wasm/regress-ubsan.js @@ -0,0 +1,19 @@ +// Copyright 2019 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +load('test/mjsunit/wasm/wasm-module-builder.js'); + +// crbug.com/933134 +(function() { + var builder = new WasmModuleBuilder(); + builder.addImportedGlobal("mod", "i32", kWasmI32); + builder.addImportedGlobal("mod", "f32", kWasmF32); + var module = new WebAssembly.Module(builder.toBuffer()); + return new WebAssembly.Instance(module, { + mod: { + i32: 1e12, + f32: 1e300, + } + }); +})(); diff --git a/deps/v8/test/mjsunit/ubsan-fuzzerbugs.js b/deps/v8/test/mjsunit/ubsan-fuzzerbugs.js index d2a21288abcfdf..7fb9e432c7ea27 100644 --- a/deps/v8/test/mjsunit/ubsan-fuzzerbugs.js +++ b/deps/v8/test/mjsunit/ubsan-fuzzerbugs.js @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// Flags: --allow-natives-syntax + // crbug.com/923466 __v_5 = [ -1073741825, -2147483648]; __v_5.sort(); @@ -17,3 +19,38 @@ new Date(2148022800000).toString(); // crbug.com/927212 assertThrows(() => (2n).toString(-2147483657), RangeError); + +// crbug.com/927894 +var typed_array = new Uint8Array(16); +typed_array.fill(0, -1.7976931348623157e+308); + +// crbug.com/927996 +var float_array = new Float32Array(1); +float_array[0] = 1e51; + +// crbug.com/930086 +(function() { + try { + // Build up a 536870910-character string (just under 2**30). + var string = "ff"; + var long_string = "0x"; + for (var i = 2; i < 29; i++) { + string = string + string; + long_string += string; + } + assertThrows(() => BigInt(long_string), SyntaxError); + } catch (e) { + /* 32-bit architectures have a lower string length limit. */ + } +})(); + +// crbug.com/932679 +(function() { + const buffer = new DataView(new ArrayBuffer(2)); + function __f_14159(buffer) { + try { return buffer.getUint16(Infinity, true); } catch(e) { return 0; } + } + __f_14159(buffer); + %OptimizeFunctionOnNextCall(__f_14159); + __f_14159(buffer); +})();