From 11d7a9d5e9ab8201a1f55f736e670685e756aa28 Mon Sep 17 00:00:00 2001
From: Ciro Spaciari <ciro.spaciari@gmail.com>
Date: Sun, 25 Aug 2024 23:16:25 -0700
Subject: [PATCH] fix(randomInt) allow negatives and improve args validation
 (#13527)

---
 src/bun.js/bindings/bindings.zig          | 11 +++++++-
 src/bun.js/node/node_crypto_binding.zig   | 24 +++++++++++++-----
 test/js/node/crypto/crypto-random.test.ts | 31 +++++++++++++++++++++++
 3 files changed, 59 insertions(+), 7 deletions(-)
 create mode 100644 test/js/node/crypto/crypto-random.test.ts

diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
index 9d5574e61f..e45de5a035 100644
--- a/src/bun.js/bindings/bindings.zig
+++ b/src/bun.js/bindings/bindings.zig
@@ -2912,6 +2912,15 @@ pub const JSGlobalObject = opaque {
         this.ERR_INVALID_ARG_TYPE("The \"{s}\" argument must be of type {s}. Received {}", .{ argname, typename, bun.fmt.quote(ty_str.slice()) }).throw();
         return .zero;
     }
+    pub fn throwInvalidArgumentRangeValue(
+        this: *JSGlobalObject,
+        argname: []const u8,
+        typename: []const u8,
+        value: i64,
+    ) JSValue {
+        this.ERR_OUT_OF_RANGE("The \"{s}\" is out of range. {s}. Received {}", .{ argname, typename, value }).throw();
+        return .zero;
+    }
 
     pub fn createNotEnoughArguments(
         this: *JSGlobalObject,
@@ -4422,7 +4431,7 @@ pub const JSValue = enum(JSValueReprInt) {
     }
 
     pub fn jsNumberFromInt64(i: i64) JSValue {
-        if (i <= std.math.maxInt(i32)) {
+        if (i <= std.math.maxInt(i32) and i >= std.math.minInt(i32)) {
             return jsNumberFromInt32(@as(i32, @intCast(i)));
         }
 
diff --git a/src/bun.js/node/node_crypto_binding.zig b/src/bun.js/node/node_crypto_binding.zig
index 93f13abeca..6add98307d 100644
--- a/src/bun.js/node/node_crypto_binding.zig
+++ b/src/bun.js/node/node_crypto_binding.zig
@@ -11,20 +11,32 @@ const assert = bun.assert;
 const EVP = Crypto.EVP;
 const PBKDF2 = EVP.PBKDF2;
 const JSValue = JSC.JSValue;
-
+const validators = @import("./util/validators.zig");
 fn randomInt(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) JSC.JSValue {
     const arguments = callframe.arguments(2).slice();
 
-    var at_least: u52 = 0;
-    var at_most: u52 = std.math.maxInt(u52);
 
     //min, max
     if (!arguments[0].isNumber()) return globalThis.throwInvalidArgumentTypeValue("min", "safe integer", arguments[0]);
     if (!arguments[1].isNumber()) return globalThis.throwInvalidArgumentTypeValue("max", "safe integer", arguments[1]);
-    at_least = arguments[0].to(u52);
-    at_most = arguments[1].to(u52);
+    const min = arguments[0].to(i64);
+    const max = arguments[1].to(i64);
+    
+    if(min > validators.NUMBER__MAX_SAFE_INTEGER or min < validators.NUMBER__MIN_SAFE_INTEGER) {
+      return globalThis.throwInvalidArgumentRangeValue("min", "It must be a safe integer type number", min);
+    }
+    if(max > validators.NUMBER__MAX_SAFE_INTEGER) {
+      return globalThis.throwInvalidArgumentRangeValue("max", "It must be a safe integer type number", max);
+    }
+    if(min >= max) {
+      return globalThis.throwInvalidArgumentRangeValue("max", "should be greater than min", max);
+    }
+    const diff = max - min;
+    if(diff > 281474976710655) {
+      return globalThis.throwInvalidArgumentRangeValue("max - min", "It must be <= 281474976710655", diff);
+    }
 
-    return JSC.JSValue.jsNumberFromUint64(std.crypto.random.intRangeLessThan(u52, at_least, at_most));
+    return JSC.JSValue.jsNumberFromInt64(std.crypto.random.intRangeLessThan(i64, min, max));
 }
 
 fn pbkdf2(
diff --git a/test/js/node/crypto/crypto-random.test.ts b/test/js/node/crypto/crypto-random.test.ts
new file mode 100644
index 0000000000..05fe3fa392
--- /dev/null
+++ b/test/js/node/crypto/crypto-random.test.ts
@@ -0,0 +1,31 @@
+import { it, expect, describe } from "bun:test";
+import { randomInt } from "crypto";
+
+describe("randomInt args validation", async () => {
+  it("default min is 0 so max should be greater than 0", () => {
+    expect(() => randomInt(-1)).toThrow(RangeError);
+    expect(() => randomInt(0)).toThrow(RangeError);
+  });
+  it("max should be >= min", () => {
+    expect(() => randomInt(1, 0)).toThrow(RangeError);
+    expect(() => randomInt(10, 5)).toThrow(RangeError);
+  });
+
+  it("we allow negative numbers", () => {
+    expect(() => randomInt(-2, -1)).not.toThrow(RangeError);
+  });
+
+  it("max/min should not be greater than Number.MAX_SAFE_INTEGER or less than Number.MIN_SAFE_INTEGER", () => {
+    expect(() => randomInt(Number.MAX_SAFE_INTEGER + 1)).toThrow(RangeError);
+    expect(() => randomInt(-Number.MAX_SAFE_INTEGER - 1, -Number.MAX_SAFE_INTEGER + 1)).toThrow(RangeError);
+  });
+
+  it("max - min should be <= 281474976710655", () => {
+    expect(() => randomInt(-2, Number.MAX_SAFE_INTEGER)).toThrow(RangeError);
+    expect(() => randomInt(-Number.MAX_SAFE_INTEGER, Number.MAX_SAFE_INTEGER)).toThrow(RangeError);
+  });
+
+  it("accept large negative numbers", () => {
+    expect(() => randomInt(-Number.MAX_SAFE_INTEGER, -Number.MAX_SAFE_INTEGER + 1)).not.toThrow(RangeError);
+  });
+});