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.
SerenityOS · Dec 7, 2022 · d37d6b3 · d37d6b3
1 parent 8f46cb8
commit d37d6b3
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 2 deletions.
diff --git a/Userland/Libraries/LibJS/Runtime/CanonicalIndex.h b/Userland/Libraries/LibJS/Runtime/CanonicalIndex.h
@@ -6,7 +6,11 @@
 
 #pragma once
 
+#include <AK/Concepts.h>
+#include <AK/NumericLimits.h>
 #include <AK/Types.h>
+#include <LibJS/Runtime/Completion.h>
+#include <LibJS/Runtime/VM.h>
 
 namespace JS {
 
@@ -24,6 +28,20 @@ class CanonicalIndex {
     {
     }
 
+    template<FloatingPoint T>
+    CanonicalIndex(Type type, T index) = delete;
+
+    template<FloatingPoint T>
+    static ThrowCompletionOr<CanonicalIndex> from_double(VM& vm, Type type, T index)
+    {
+        if (index < static_cast<double>(NumericLimits<u32>::min()))
+            return vm.throw_completion<RangeError>(ErrorType::TypedArrayInvalidIntegerIndex, index);
+        if (index > static_cast<double>(NumericLimits<u32>::max()))
+            return vm.throw_completion<RangeError>(ErrorType::TypedArrayInvalidIntegerIndex, index);
+
+        return CanonicalIndex { type, static_cast<u32>(index) };
+    }
+
     u32 as_index() const
     {
         VERIFY(is_index());

diff --git a/Userland/Libraries/LibJS/Runtime/TypedArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/TypedArrayPrototype.cpp
@@ -1197,7 +1197,8 @@ static ThrowCompletionOr<void> 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<RangeError>(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
@@ -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 => {