From b2e68430552a5fcf3152f16983e90f2b9730d6d4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 18 May 2024 10:58:11 +0200 Subject: [PATCH] LibJS+AK: Fix integer overflow UB on (any Int32 - -2147483648) It wasn't safe to use addition_would_overflow(a, -b) to check if subtraction (a - b) would overflow, since it doesn't cover this case. I don't know why we didn't have subtraction_would_overflow(), so this patch adds it. :^) --- AK/Checked.h | 16 ++++++++++++++++ .../Libraries/LibJS/Bytecode/Interpreter.cpp | 2 +- .../LibJS/Tests/math/integer-overflow-basic.js | 10 ++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 Userland/Libraries/LibJS/Tests/math/integer-overflow-basic.js diff --git a/AK/Checked.h b/AK/Checked.h index e07216bf3cf..e31f5601f19 100644 --- a/AK/Checked.h +++ b/AK/Checked.h @@ -359,6 +359,22 @@ public: #endif } + template + [[nodiscard]] static constexpr bool subtraction_would_overflow(U u, V v) + { +#if __has_builtin(__builtin_sub_overflow_p) + return __builtin_sub_overflow_p(u, v, (T)0); +#elif __has_builtin(__builtin_sub_overflow) + T result; + return __builtin_sub_overflow(u, v, &result); +#else + Checked checked; + checked = u; + checked -= v; + return checked.has_overflow(); +#endif + } + template static constexpr T saturating_add(U a, V b) { diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 0b206058e8d..952194df977 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -948,7 +948,7 @@ ThrowCompletionOr Sub::execute_impl(Bytecode::Interpreter& interpreter) co if (lhs.is_number() && rhs.is_number()) { if (lhs.is_int32() && rhs.is_int32()) { - if (!Checked::addition_would_overflow(lhs.as_i32(), -rhs.as_i32())) { + if (!Checked::subtraction_would_overflow(lhs.as_i32(), rhs.as_i32())) { interpreter.set(m_dst, Value(lhs.as_i32() - rhs.as_i32())); return {}; } diff --git a/Userland/Libraries/LibJS/Tests/math/integer-overflow-basic.js b/Userland/Libraries/LibJS/Tests/math/integer-overflow-basic.js new file mode 100644 index 00000000000..d4729711ec6 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/math/integer-overflow-basic.js @@ -0,0 +1,10 @@ +test("basic integer overflow correctness", () => { + expect(2147483647 + 1).toBe(2147483648); + expect(2147483648 - 1).toBe(2147483647); + expect(0 - 2147483647).toBe(-2147483647); + expect(0 - 2147483648).toBe(-2147483648); + expect(0 - -2147483647).toBe(2147483647); + expect(0 - -2147483648).toBe(2147483648); + expect(0 + -2147483647).toBe(-2147483647); + expect(0 + -2147483648).toBe(-2147483648); +});