fs: validate file mode from cpp

PR-URL: #52050
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: anonrig

lib/fs.js

@@ -108,7 +108,6 @@ const {
   getOptions,
   getValidatedFd,
   getValidatedPath,
-  getValidMode,
   handleErrorFromBinding,
   preprocessSymlinkDestination,
   Stats,
@@ -233,7 +232,6 @@ function access(path, mode, callback) {
   }
 
   path = getValidatedPath(path);
-  mode = getValidMode(mode, 'access');
   callback = makeCallback(callback);
 
   const req = new FSReqCallback();
@@ -250,8 +248,6 @@ function access(path, mode, callback) {
  */
 function accessSync(path, mode) {
   path = getValidatedPath(path);
-  mode = getValidMode(mode, 'access');
-
   binding.access(pathModule.toNamespacedPath(path), mode);
 }
 
@@ -2984,7 +2980,6 @@ function copyFile(src, dest, mode, callback) {
 
   src = pathModule.toNamespacedPath(src);
   dest = pathModule.toNamespacedPath(dest);
-  mode = getValidMode(mode, 'copyFile');
   callback = makeCallback(callback);
 
   const req = new FSReqCallback();
@@ -3007,7 +3002,7 @@ function copyFileSync(src, dest, mode) {
   binding.copyFile(
     pathModule.toNamespacedPath(src),
     pathModule.toNamespacedPath(dest),
-    getValidMode(mode, 'copyFile'),
+    mode,
   );
 }
 

lib/internal/fs/promises.js

@@ -59,7 +59,6 @@ const {
   getStatFsFromBinding,
   getStatsFromBinding,
   getValidatedPath,
-  getValidMode,
   preprocessSymlinkDestination,
   stringToFlags,
   stringToSymlinkType,
@@ -600,7 +599,6 @@ async function readFileHandle(filehandle, options) {
 async function access(path, mode = F_OK) {
   path = getValidatedPath(path);
 
-  mode = getValidMode(mode, 'access');
   return await PromisePrototypeThen(
     binding.access(pathModule.toNamespacedPath(path), mode, kUsePromises),
     undefined,
@@ -618,7 +616,6 @@ async function cp(src, dest, options) {
 async function copyFile(src, dest, mode) {
   src = getValidatedPath(src, 'src');
   dest = getValidatedPath(dest, 'dest');
-  mode = getValidMode(mode, 'copyFile');
   return await PromisePrototypeThen(
     binding.copyFile(pathModule.toNamespacedPath(src),
                      pathModule.toNamespacedPath(dest),

lib/internal/fs/utils.js

@@ -1000,7 +1000,6 @@ module.exports = {
   getOptions,
   getValidatedFd,
   getValidatedPath,
-  getValidMode,
   handleErrorFromBinding,
   possiblyTransformPath,
   preprocessSymlinkDestination,

src/node_file.cc

@@ -38,6 +38,7 @@
 #include "req_wrap-inl.h"
 #include "stream_base-inl.h"
 #include "string_bytes.h"
+#include "uv.h"
 
 #if defined(__MINGW32__) || defined(_MSC_VER)
 # include <io.h>
@@ -950,10 +951,12 @@ void Access(const FunctionCallbackInfo<Value>& args) {
   HandleScope scope(isolate);
 
   const int argc = args.Length();
-  CHECK_GE(argc, 2);
+  CHECK_GE(argc, 2);  // path, mode
 
-  CHECK(args[1]->IsInt32());
-  int mode = args[1].As<Int32>()->Value();
+  int mode;
+  if (!GetValidFileMode(env, args[1], UV_FS_ACCESS).To(&mode)) {
+    return;
+  }
 
   BufferValue path(isolate, args[0]);
   CHECK_NOT_NULL(*path);
@@ -1982,7 +1985,12 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = env->isolate();
 
   const int argc = args.Length();
-  CHECK_GE(argc, 3);
+  CHECK_GE(argc, 3);  // src, dest, flags
+
+  int flags;
+  if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&flags)) {
+    return;
+  }
 
   BufferValue src(isolate, args[0]);
   CHECK_NOT_NULL(*src);
@@ -1994,9 +2002,6 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
   THROW_IF_INSUFFICIENT_PERMISSIONS(
       env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
 
-  CHECK(args[2]->IsInt32());
-  const int flags = args[2].As<Int32>()->Value();
-
   if (argc > 3) {  // copyFile(src, dest, flags, req)
     FSReqBase* req_wrap_async = GetReqWrap(args, 3);
     FS_ASYNC_TRACE_BEGIN2(UV_FS_COPYFILE,

src/util.cc

@@ -21,6 +21,7 @@
 
 #include "util.h"  // NOLINT(build/include_inline)
 #include <cmath>
+#include <cstdint>
 #include "util-inl.h"
 
 #include "debug_utils-inl.h"
@@ -31,7 +32,6 @@
 #include "node_snapshot_builder.h"
 #include "node_v8_platform-inl.h"
 #include "string_bytes.h"
-#include "uv.h"
 #include "v8-value.h"
 
 #ifdef _WIN32
@@ -56,6 +56,31 @@
 
 static std::atomic_int seq = {0};  // Sequence number for diagnostic filenames.
 
+// F_OK etc. constants
+#ifdef _WIN32
+#include "uv.h"
+#else
+#include <unistd.h>
+#endif
+
+// The access modes can be any of F_OK, R_OK, W_OK or X_OK. Some might not be
+// available on specific systems. They can be used in combination as well
+// (F_OK | R_OK | W_OK | X_OK).
+constexpr int kMaximumAccessMode = F_OK | W_OK | R_OK | X_OK;
+constexpr int kMinimumAccessMode = std::min({F_OK, W_OK, R_OK, X_OK});
+
+constexpr int kDefaultCopyMode = 0;
+// The copy modes can be any of UV_FS_COPYFILE_EXCL, UV_FS_COPYFILE_FICLONE or
+// UV_FS_COPYFILE_FICLONE_FORCE. They can be used in combination as well
+// (US_FS_COPYFILE_EXCL | US_FS_COPYFILE_FICLONE |
+// US_FS_COPYFILE_FICLONE_FORCE).
+constexpr int kMinimumCopyMode = std::min({kDefaultCopyMode,
+                                           UV_FS_COPYFILE_EXCL,
+                                           UV_FS_COPYFILE_FICLONE,
+                                           UV_FS_COPYFILE_FICLONE_FORCE});
+constexpr int kMaximumCopyMode =
+    UV_FS_COPYFILE_EXCL | UV_FS_COPYFILE_FICLONE | UV_FS_COPYFILE_FICLONE_FORCE;
+
 namespace node {
 
 using v8::ArrayBuffer;
@@ -787,4 +812,49 @@ v8::Maybe<int32_t> GetValidatedFd(Environment* env,
   return v8::Just(static_cast<int32_t>(fd));
 }
 
+v8::Maybe<int> GetValidFileMode(Environment* env,
+                                v8::Local<v8::Value> input,
+                                uv_fs_type type) {
+  // Allow only int32 or null/undefined values.
+  if (input->IsNumber()) {
+    // We cast the input to v8::Number to avoid overflows.
+    auto num = input.As<v8::Number>()->Value();
+
+    // Handle infinity and NaN values
+    if (std::isinf(num) || std::isnan(num)) {
+      THROW_ERR_OUT_OF_RANGE(env, "mode is out of range");
+      return v8::Nothing<int>();
+    }
+  } else if (!input->IsNullOrUndefined()) {
+    THROW_ERR_INVALID_ARG_TYPE(env, "mode must be int32 or null/undefined");
+    return v8::Nothing<int>();
+  }
+
+  int min = kMinimumAccessMode;
+  int max = kMaximumAccessMode;
+  int def = F_OK;
+
+  CHECK(type == UV_FS_ACCESS || type == UV_FS_COPYFILE);
+
+  if (type == UV_FS_COPYFILE) {
+    min = kMinimumCopyMode;
+    max = kMaximumCopyMode;
+    def = input->IsNullOrUndefined() ? kDefaultCopyMode
+                                     : input.As<v8::Int32>()->Value();
+  }
+
+  if (input->IsNullOrUndefined()) {
+    return v8::Just(def);
+  }
+
+  const int mode = input.As<v8::Int32>()->Value();
+  if (mode < min || mode > max) {
+    THROW_ERR_OUT_OF_RANGE(
+        env, "mode is out of range: >= %d && <= %d", min, max);
+    return v8::Nothing<int>();
+  }
+
+  return v8::Just(mode);
+}
+
 }  // namespace node

src/util.h

@@ -24,6 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include "uv.h"
 #include "v8.h"
 
 #include "node.h"
@@ -1027,6 +1028,9 @@ std::string DetermineSpecificErrorType(Environment* env,
                                        v8::Local<v8::Value> input);
 
 v8::Maybe<int32_t> GetValidatedFd(Environment* env, v8::Local<v8::Value> input);
+v8::Maybe<int> GetValidFileMode(Environment* env,
+                                v8::Local<v8::Value> input,
+                                uv_fs_type type);
 
 }  // namespace node