fs: validate copyFile paths in the existing C++ binding

lib/fs.js

@@ -3659,8 +3659,6 @@ function copyFile(src, dest, mode, callback) {
   const h = vfsState.handlers;
   if (h !== null && vfsVoid(h.copyFile(src, dest, mode), callback)) return;
 
-  src = getValidatedPath(src, 'src');
-  dest = getValidatedPath(dest, 'dest');
   callback = makeCallback(callback);
 
   const req = new FSReqCallback();
@@ -3682,11 +3680,7 @@ function copyFileSync(src, dest, mode) {
     const result = h.copyFileSync(src, dest, mode);
     if (result !== undefined) return;
   }
-  binding.copyFile(
-    getValidatedPath(src, 'src'),
-    getValidatedPath(dest, 'dest'),
-    mode,
-  );
+  binding.copyFile(src, dest, mode);
 }
 
 /**

lib/internal/fs/promises.js

@@ -1335,12 +1335,7 @@ async function copyFile(src, dest, mode) {
     if (promise !== undefined) { await promise; return; }
   }
   return await PromisePrototypeThen(
-    binding.copyFile(
-      getValidatedPath(src, 'src'),
-      getValidatedPath(dest, 'dest'),
-      mode,
-      kUsePromises,
-    ),
+    binding.copyFile(src, dest, mode, kUsePromises),
     undefined,
     handleErrorFromBinding,
   );

src/node_file.cc

@@ -46,6 +46,7 @@
 #include <errno.h>
 #include <cerrno>
 #include <cstdio>
+#include <cstring>
 #include <filesystem>
 
 #if defined(__MINGW32__) || defined(_MSC_VER)
@@ -77,6 +78,7 @@ using v8::Local;
 using v8::LocalVector;
 using v8::Maybe;
 using v8::MaybeLocal;
+using v8::NewStringType;
 using v8::Nothing;
 using v8::Number;
 using v8::Object;
@@ -2416,24 +2418,172 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+// Matches lib/internal/url.js isURL(): duck-type WHATWG URL objects vs
+// legacy url.parse() results (which have `auth` and `path` properties).
+static bool IsUrlLike(Environment* env, Local<Value> value) {
+  if (!value->IsObject() || value->IsUint8Array()) {
+    return false;
+  }
+
+  Isolate* isolate = env->isolate();
+  Local<Context> context = env->context();
+  Local<Object> obj = value.As<Object>();
+
+  Local<Value> href;
+  if (!obj->Get(context, env->href_string()).ToLocal(&href)) {
+    return false;
+  }
+  if (!href->BooleanValue(isolate)) {
+    return false;
+  }
+
+  Local<Value> protocol;
+  if (!obj->Get(context, env->protocol_string()).ToLocal(&protocol)) {
+    return false;
+  }
+  if (!protocol->BooleanValue(isolate)) {
+    return false;
+  }
+
+  Local<Value> auth;
+  if (!obj->Get(context, FIXED_ONE_BYTE_STRING(isolate, "auth"))
+           .ToLocal(&auth)) {
+    return false;
+  }
+
+  Local<Value> path;
+  if (!obj->Get(context, env->path_string()).ToLocal(&path)) {
+    return false;
+  }
+
+  return auth->IsUndefined() && path->IsUndefined();
+}
+
+static bool ContainsNul(const BufferValue& path) {
+  return path.length() > 0 &&
+         std::memchr(path.out(), '\0', path.length()) != nullptr;
+}
+
+// C++ equivalent of getValidatedPath(value, propName), used by CopyFile for
+// the sync, callback, and promises paths. Accepts string, Uint8Array/Buffer,
+// or WHATWG URL, rejects embedded NUL bytes, and converts file: URLs to
+// paths. Returns an empty MaybeLocal and throws on failure.
+static MaybeLocal<Value> GetValidatedPath(Environment* env,
+                                          Local<Value> input,
+                                          const char* prop_name) {
+  Isolate* isolate = env->isolate();
+
+  if (input->IsString() || input->IsUint8Array()) {
+    return input;
+  }
+
+  if (IsUrlLike(env, input)) {
+    Local<Context> context = env->context();
+    Local<Value> href;
+    if (!input.As<Object>()->Get(context, env->href_string()).ToLocal(&href)) {
+      return MaybeLocal<Value>();
+    }
+
+    Utf8Value href_utf8(isolate, href);
+    auto parsed = ada::parse<ada::url_aggregator>(href_utf8.ToStringView());
+    if (!parsed) {
+      url::ThrowInvalidURL(env, href_utf8.ToStringView(), std::nullopt);
+      return MaybeLocal<Value>();
+    }
+
+    // Match lib/internal/url.js fileURLToPath(): non-file schemes throw
+    // ERR_INVALID_URL_SCHEME with the same message as JS (`file`, not `file:`).
+    if (parsed->type != ada::scheme::FILE) {
+      THROW_ERR_INVALID_URL_SCHEME(isolate, "The URL must be of scheme file");
+      return MaybeLocal<Value>();
+    }
+
+    std::optional<std::string> file_path = url::FileURLToPath(env, *parsed);
+    if (!file_path.has_value()) {
+      return MaybeLocal<Value>();
+    }
+
+    if (file_path->find('\0') != std::string::npos) {
+      THROW_ERR_INVALID_ARG_VALUE(
+          env,
+          "The argument '%s' must be a string, Uint8Array, or URL "
+          "without null bytes. Received %s",
+          prop_name,
+          DetermineSpecificErrorType(env, input));
+      return MaybeLocal<Value>();
+    }
+
+    Local<String> path_string;
+    if (!String::NewFromUtf8(isolate,
+                             file_path->data(),
+                             NewStringType::kNormal,
+                             static_cast<int>(file_path->size()))
+             .ToLocal(&path_string)) {
+      return MaybeLocal<Value>();
+    }
+    return path_string;
+  }
+
+  if (isolate->HasPendingException()) {
+    return MaybeLocal<Value>();
+  }
+
+  THROW_ERR_INVALID_ARG_TYPE(
+      env,
+      "The \"%s\" argument must be of type string or an instance of "
+      "Buffer or URL. Received %s",
+      prop_name,
+      DetermineSpecificErrorType(env, input));
+  return MaybeLocal<Value>();
+}
+
+// Shared by the sync, callback, and promises copyFile paths. Path validation
+// (string / Uint8Array / file: URL, NUL checks) happens here so JS callers
+// can pass the original arguments through.
 static void CopyFile(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
 
   const int argc = args.Length();
-  CHECK_GE(argc, 3);  // src, dest, flags
+  CHECK_GE(argc, 3);  // src, dest, flags[, req]
 
-  int flags;
-  if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&flags)) {
+  Local<Value> src_val;
+  if (!GetValidatedPath(env, args[0], "src").ToLocal(&src_val)) {
+    return;
+  }
+  Local<Value> dest_val;
+  if (!GetValidatedPath(env, args[1], "dest").ToLocal(&dest_val)) {
     return;
   }
 
-  BufferValue src(isolate, args[0]);
+  BufferValue src(isolate, src_val);
   CHECK_NOT_NULL(*src);
-  ToNamespacedPath(env, &src);
+  if (ContainsNul(src)) {
+    THROW_ERR_INVALID_ARG_VALUE(
+        env,
+        "The argument 'src' must be a string, Uint8Array, or URL "
+        "without null bytes. Received %s",
+        DetermineSpecificErrorType(env, args[0]));
+    return;
+  }
 
-  BufferValue dest(isolate, args[1]);
+  BufferValue dest(isolate, dest_val);
   CHECK_NOT_NULL(*dest);
+  if (ContainsNul(dest)) {
+    THROW_ERR_INVALID_ARG_VALUE(
+        env,
+        "The argument 'dest' must be a string, Uint8Array, or URL "
+        "without null bytes. Received %s",
+        DetermineSpecificErrorType(env, args[1]));
+    return;
+  }
+
+  int flags;
+  if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&flags)) {
+    return;
+  }
+
+  ToNamespacedPath(env, &src);
   ToNamespacedPath(env, &dest);
 
   if (argc > 3) {  // copyFile(src, dest, flags, req)

src/node_url.cc

@@ -688,7 +688,7 @@ std::optional<std::string> FileURLToPath(Environment* env,
   if (hostname.size() > 0) {
     THROW_ERR_INVALID_FILE_URL_HOST(
         env->isolate(),
-        "File URL host must be \"localhost\" or empty on ",
+        "File URL host must be \"localhost\" or empty on %s",
         std::string(per_process::metadata.platform));
     return std::nullopt;
   }

test/parallel/test-fs-copyfile.js

@@ -10,6 +10,7 @@ const {
   UV_ENOENT,
   UV_EEXIST
 } = internalBinding('uv');
+const { pathToFileURL } = require('url');
 const src = fixtures.path('a.js');
 const dest = tmpdir.resolve('copyfile.out');
 const {
@@ -55,6 +56,22 @@ verify(src, dest);
 fs.copyFileSync(src, dest, 0);
 verify(src, dest);
 
+// Verify Buffer and file: URL paths.
+{
+  const destBuf = tmpdir.resolve('copyfile.buffer');
+  fs.copyFileSync(Buffer.from(src), Buffer.from(destBuf));
+  verify(src, destBuf);
+
+  const destUrl = tmpdir.resolve('copyfile.url');
+  fs.copyFileSync(pathToFileURL(src), pathToFileURL(destUrl));
+  verify(src, destUrl);
+
+  const destU8 = tmpdir.resolve('copyfile.uint8');
+  fs.copyFileSync(new Uint8Array(Buffer.from(src)),
+                  new Uint8Array(Buffer.from(destU8)));
+  verify(src, destU8);
+}
+
 // Verify that UV_FS_COPYFILE_FICLONE can be used.
 fs.unlinkSync(dest);
 fs.copyFileSync(src, dest, UV_FS_COPYFILE_FICLONE);
@@ -142,6 +159,51 @@ assert.throws(() => {
   );
 });
 
+assert.throws(() => {
+  fs.copyFileSync(new URL('http://example.com/a'), dest);
+}, {
+  code: 'ERR_INVALID_URL_SCHEME',
+  name: 'TypeError',
+  message: 'The URL must be of scheme file',
+});
+
+assert.throws(() => {
+  fs.copyFile(new URL('http://example.com/a'), dest, common.mustNotCall());
+}, {
+  code: 'ERR_INVALID_URL_SCHEME',
+  name: 'TypeError',
+  message: 'The URL must be of scheme file',
+});
+
+if (common.isWindows) {
+  ['%2f', '%2F', '%5c', '%5C'].forEach((i) => {
+    assert.throws(
+      () => fs.copyFileSync(new URL(`file:///c:/tmp/${i}`), dest),
+      {
+        code: 'ERR_INVALID_FILE_URL_PATH',
+        name: 'TypeError',
+      }
+    );
+  });
+} else {
+  ['%2f', '%2F'].forEach((i) => {
+    assert.throws(
+      () => fs.copyFileSync(new URL(`file:///c:/tmp/${i}`), dest),
+      {
+        code: 'ERR_INVALID_FILE_URL_PATH',
+        name: 'TypeError',
+      }
+    );
+  });
+  assert.throws(
+    () => fs.copyFileSync(new URL('file://hostname/a/b/c'), dest),
+    {
+      code: 'ERR_INVALID_FILE_URL_HOST',
+      name: 'TypeError',
+    }
+  );
+}
+
 assert.throws(() => {
   fs.copyFileSync(src, dest, 'r');
 }, {

typings/internalBinding/fs.d.ts

@@ -72,9 +72,10 @@ declare namespace InternalFSBinding {
   function close(fd: number, req: FSReqCallback): void;
   function close(fd: number): void;
 
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: FSReqCallback): void;
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: undefined, ctx: FSSyncContext): void;
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
+  function copyFile(src: unknown, dest: unknown, mode?: unknown): void;
+  function copyFile(src: unknown, dest: unknown, mode: unknown, req: FSReqCallback): void;
+  function copyFile(src: unknown, dest: unknown, mode: unknown, req: undefined, ctx: FSSyncContext): void;
+  function copyFile(src: unknown, dest: unknown, mode: unknown, usePromises: typeof kUsePromises): Promise<void>;
 
   function cpSyncCheckPaths(src: StringOrBuffer, dest: StringOrBuffer, dereference: boolean, recursive: boolean): void;
   function cpSyncOverrideFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, preserveTimestamps: boolean): void;