From d37d6b3479aece9be1cb9fc5492e1c8e0f6a4cb8 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Wed, 7 Dec 2022 08:11:05 -0500 Subject: [PATCH] LibJS: Protect CanonicalIndex against double-to-integer overflow Explicitly disallow constructing a CanonicalIndex from a floating point type without going through a factory method that will throw when the provided index cannot fit in a u32. --- .../Libraries/LibJS/Runtime/CanonicalIndex.h | 18 +++++++++ .../LibJS/Runtime/TypedArrayPrototype.cpp | 5 ++- .../TypedArray/TypedArray.prototype.with.js | 38 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/CanonicalIndex.h b/Userland/Libraries/LibJS/Runtime/CanonicalIndex.h index be65059ba9..9260a4405f 100644 --- a/Userland/Libraries/LibJS/Runtime/CanonicalIndex.h +++ b/Userland/Libraries/LibJS/Runtime/CanonicalIndex.h @@ -6,7 +6,11 @@ #pragma once +#include +#include #include +#include +#include namespace JS { @@ -24,6 +28,20 @@ public: { } + template + CanonicalIndex(Type type, T index) = delete; + + template + static ThrowCompletionOr from_double(VM& vm, Type type, T index) + { + if (index < static_cast(NumericLimits::min())) + return vm.throw_completion(ErrorType::TypedArrayInvalidIntegerIndex, index); + if (index > static_cast(NumericLimits::max())) + return vm.throw_completion(ErrorType::TypedArrayInvalidIntegerIndex, index); + + return CanonicalIndex { type, static_cast(index) }; + } + u32 as_index() const { VERIFY(is_index()); diff --git a/Userland/Libraries/LibJS/Runtime/TypedArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/TypedArrayPrototype.cpp index a7307920c0..d6bff2e42e 100644 --- a/Userland/Libraries/LibJS/Runtime/TypedArrayPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/TypedArrayPrototype.cpp @@ -1197,7 +1197,8 @@ static ThrowCompletionOr set_typed_array_from_array_like(VM& vm, TypedArra auto value = TRY(src->get(k)); // c. Let targetIndex be 𝔽(targetOffset + k). - CanonicalIndex target_index(CanonicalIndex::Type::Index, target_offset + k); + // NOTE: We verify above that target_offset + source_length is valid, so this cannot fail. + auto target_index = MUST(CanonicalIndex::from_double(vm, CanonicalIndex::Type::Index, target_offset + k)); // d. Perform ? IntegerIndexedElementSet(target, targetIndex, value). // FIXME: This is very awkward. @@ -1679,7 +1680,7 @@ JS_DEFINE_NATIVE_FUNCTION(TypedArrayPrototype::with) value = TRY(value.to_number(vm)); // 9. If ! IsValidIntegerIndex(O, 𝔽(actualIndex)) is false, throw a RangeError exception. - if (!is_valid_integer_index(*typed_array, CanonicalIndex(CanonicalIndex::Type::Index, actual_index))) + if (!is_valid_integer_index(*typed_array, TRY(CanonicalIndex::from_double(vm, CanonicalIndex::Type::Index, actual_index)))) return vm.throw_completion(ErrorType::TypedArrayInvalidIntegerIndex, actual_index); // 10. Let A be ? TypedArrayCreateSameType(O, « 𝔽(len) »). diff --git a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.prototype.with.js b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.prototype.with.js index 9668e30a03..8c501331f9 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.prototype.with.js +++ b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.prototype.with.js @@ -12,6 +12,44 @@ const TYPED_ARRAYS = [ const BIGINT_TYPED_ARRAYS = [BigUint64Array, BigInt64Array]; +describe("errors", () => { + test("index out of range", () => { + TYPED_ARRAYS.forEach(T => { + const array = new T([1, 2, 3]); + + expect(() => { + array.with(3, 10); + }).toThrowWithMessage(RangeError, "Invalid integer index: 3"); + + expect(() => { + array.with(-4, 10); + }).toThrowWithMessage(RangeError, "Invalid integer index: -1"); + }); + }); + + test("invalid index", () => { + TYPED_ARRAYS.forEach(T => { + const array = new T([1, 2, 3]); + + expect(() => { + array.with(2 ** 53, 10); + }).toThrowWithMessage(RangeError, "Invalid integer index: 9007199254740992"); + + expect(() => { + array.with(-(2 ** 53), 10); + }).toThrowWithMessage(RangeError, "Invalid integer index: -9007199254740989"); + + expect(() => { + array.with(Infinity, 10); + }).toThrowWithMessage(RangeError, "Invalid integer index: inf"); + + expect(() => { + array.with(-Infinity, 10); + }).toThrowWithMessage(RangeError, "Invalid integer index: -inf"); + }); + }); +}); + describe("normal behavior", () => { test("length is 2", () => { TYPED_ARRAYS.forEach(T => {