node:fs: fix assertion when chown is passed non-numbers (#13113)

oven-sh · Aug 7, 2024 · c63c55c · c63c55c
1 parent 6d09772
commit c63c55c
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 15 deletions.
diff --git a/src/bun.js/bindings/ErrorCode.cpp b/src/bun.js/bindings/ErrorCode.cpp
@@ -65,6 +65,7 @@ static JSC::JSObject* createErrorPrototype(JSC::VM& vm, JSC::JSGlobalObject* glo
 }
 
 extern "C" JSC::EncodedJSValue Bun__ERR_INVALID_ARG_TYPE(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue val_arg_name, JSC::EncodedJSValue val_expected_type, JSC::EncodedJSValue val_actual_value);
+extern "C" JSC::EncodedJSValue Bun__ERR_INVALID_ARG_TYPE_static(JSC::JSGlobalObject* globalObject, const ZigString* val_arg_name, const ZigString* val_expected_type, JSC::EncodedJSValue val_actual_value);
 extern "C" JSC::EncodedJSValue Bun__ERR_MISSING_ARGS(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue arg1, JSC::EncodedJSValue arg2, JSC::EncodedJSValue arg3);
 extern "C" JSC::EncodedJSValue Bun__ERR_IPC_CHANNEL_CLOSED(JSC::JSGlobalObject* globalObject);
 
@@ -224,6 +225,23 @@ extern "C" JSC::EncodedJSValue Bun__ERR_INVALID_ARG_TYPE(JSC::JSGlobalObject* gl
     auto message = makeString("The \""_s, arg_name, "\" argument must be of type "_s, expected_type, ". Received "_s, actual_value);
     return JSValue::encode(createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, message));
 }
+extern "C" JSC::EncodedJSValue Bun__ERR_INVALID_ARG_TYPE_static(JSC::JSGlobalObject* globalObject, const ZigString* val_arg_name, const ZigString* val_expected_type, JSC::EncodedJSValue val_actual_value)
+{
+    JSC::VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    auto arg_name = std::span<const unsigned char>(val_arg_name->ptr, val_arg_name->len);
+    ASSERT(WTF::charactersAreAllASCII(arg_name));
+
+    auto expected_type = std::span<const unsigned char>(val_expected_type->ptr, val_expected_type->len);
+    ASSERT(WTF::charactersAreAllASCII(expected_type));
+
+    auto actual_value = JSValueToStringSafe(globalObject, JSValue::decode(val_actual_value));
+    RETURN_IF_EXCEPTION(scope, {});
+
+    auto message = makeString("The \""_s, arg_name, "\" argument must be of type "_s, expected_type, ". Received "_s, actual_value);
+    return JSValue::encode(createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, message));
+}
 
 JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_OUT_OF_RANGE, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
 {

diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
@@ -93,21 +93,6 @@ pub const JSObject = extern struct {
         });
     }
 
-    extern fn Bun__ERR_INVALID_ARG_TYPE(*JSGlobalObject, JSValue, JSValue, JSValue) JSValue;
-    pub fn ERR_INVALID_ARG_TYPE(this: *JSGlobalObject, arg_name: JSValue, etype: JSValue, atype: JSValue) JSValue {
-        return Bun__ERR_INVALID_ARG_TYPE(this, arg_name, etype, atype);
-    }
-
-    extern fn Bun__ERR_MISSING_ARGS(*JSGlobalObject, JSValue, JSValue, JSValue) JSValue;
-    pub fn ERR_MISSING_ARGS(this: *JSGlobalObject, arg1: JSValue, arg2: JSValue, arg3: JSValue) JSValue {
-        return Bun__ERR_MISSING_ARGS(this, arg1, arg2, arg3);
-    }
-
-    extern fn Bun__ERR_IPC_CHANNEL_CLOSED(*JSGlobalObject) JSValue;
-    pub fn ERR_IPC_CHANNEL_CLOSED(this: *JSGlobalObject) JSValue {
-        return Bun__ERR_IPC_CHANNEL_CLOSED(this);
-    }
-
     pub const Extern = [_][]const u8{
         "putRecord",
         "getArrayLength",
@@ -3225,6 +3210,12 @@ pub const JSGlobalObject = opaque {
         if (bun.Environment.allow_assert) this.bunVM().assertOnJSThread();
     }
 
+    extern fn Bun__ERR_INVALID_ARG_TYPE_static(*JSGlobalObject, *const ZigString, *const ZigString, JSValue) JSValue;
+    /// Caller asserts 'arg_name' and 'etype' are utf-8 literals.
+    pub fn ERR_INVALID_ARG_TYPE_static(this: *JSGlobalObject, arg_name: *const ZigString, etype: *const ZigString, atype: JSValue) JSValue {
+        return Bun__ERR_INVALID_ARG_TYPE_static(this, arg_name, etype, atype);
+    }
+
     pub usingnamespace @import("ErrorCode").JSGlobalObjectExtensions;
 };
 

diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig
@@ -1444,6 +1444,14 @@ pub const Arguments = struct {
                 };
 
                 arguments.eat();
+                if (!uid_value.isNumber()) {
+                    ctx.throwValue(ctx.ERR_INVALID_ARG_TYPE_static(
+                        JSC.ZigString.static("uid"),
+                        JSC.ZigString.static("number"),
+                        uid_value,
+                    ));
+                    return null;
+                }
                 break :brk @as(uid_t, @intCast(uid_value.toInt32()));
             };
 
@@ -1461,6 +1469,14 @@ pub const Arguments = struct {
                 };
 
                 arguments.eat();
+                if (!gid_value.isNumber()) {
+                    ctx.throwValue(ctx.ERR_INVALID_ARG_TYPE_static(
+                        JSC.ZigString.static("gid"),
+                        JSC.ZigString.static("number"),
+                        gid_value,
+                    ));
+                    return null;
+                }
                 break :brk @as(gid_t, @intCast(gid_value.toInt32()));
             };
 

diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts
@@ -3285,3 +3285,8 @@ it("promises.appendFile should accept a FileHandle", async () => {
   await fs.promises.appendFile(file, "data");
   expect(await Bun.file(x_path).text()).toBe("datadata");
 });
+
+it("chown should verify its arguments", () => {
+  expect(() => fs.chown("doesnt-matter.txt", "a", 0)).toThrowWithCode(TypeError, "ERR_INVALID_ARG_TYPE");
+  expect(() => fs.chown("doesnt-matter.txt", 0, "a")).toThrowWithCode(TypeError, "ERR_INVALID_ARG_TYPE");
+});