From f88f1f17cda45c75c7a58c3ffe8feb783f0c6b5a Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Mon, 11 Mar 2024 21:09:00 -0400 Subject: [PATCH] fs: validate file mode from cpp --- lib/fs.js | 7 +--- lib/internal/fs/promises.js | 3 -- lib/internal/fs/utils.js | 1 - src/node_file.cc | 17 +++++---- src/util.cc | 72 ++++++++++++++++++++++++++++++++++++- src/util.h | 4 +++ 6 files changed, 87 insertions(+), 17 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 0377e0341e3efd..2377ae68c6cfff 100644 --- a/lib/fs.js +++ b/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, ); } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index d6de9c8e9b6b93..676583ffea5d00 100644 --- a/lib/internal/fs/promises.js +++ b/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), diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 087d27f2db7bb9..968f011f668710 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -1000,7 +1000,6 @@ module.exports = { getOptions, getValidatedFd, getValidatedPath, - getValidMode, handleErrorFromBinding, possiblyTransformPath, preprocessSymlinkDestination, diff --git a/src/node_file.cc b/src/node_file.cc index a3e80898cde6b6..ddbd104e3f178c 100644 --- a/src/node_file.cc +++ b/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 @@ -950,10 +951,12 @@ void Access(const FunctionCallbackInfo& 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()->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,7 @@ static void CopyFile(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); const int argc = args.Length(); - CHECK_GE(argc, 3); + CHECK_GE(argc, 3); // src, dest, flags BufferValue src(isolate, args[0]); CHECK_NOT_NULL(*src); @@ -1994,8 +1997,10 @@ static void CopyFile(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView()); - CHECK(args[2]->IsInt32()); - const int flags = args[2].As()->Value(); + int flags; + if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&flags)) { + return; + } if (argc > 3) { // copyFile(src, dest, flags, req) FSReqBase* req_wrap_async = GetReqWrap(args, 3); diff --git a/src/util.cc b/src/util.cc index 22b17c6f5f6a08..9c90a31fec807d 100644 --- a/src/util.cc +++ b/src/util.cc @@ -21,6 +21,7 @@ #include "util.h" // NOLINT(build/include_inline) #include +#include #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 +#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 GetValidatedFd(Environment* env, return v8::Just(static_cast(fd)); } +v8::Maybe GetValidFileMode(Environment* env, + v8::Local 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()->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(); + } + } else if (!input->IsNullOrUndefined()) { + THROW_ERR_INVALID_ARG_TYPE(env, "mode must be int32 or null/undefined"); + return v8::Nothing(); + } + + 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()->Value(); + } + + if (input->IsNullOrUndefined()) { + return v8::Just(def); + } + + const int mode = input.As()->Value(); + if (mode < min || mode > max) { + THROW_ERR_OUT_OF_RANGE( + env, "mode is out of range: >= %d && <= %d", min, max); + return v8::Nothing(); + } + + return v8::Just(mode); +} + } // namespace node diff --git a/src/util.h b/src/util.h index a9d1ec039e8517..778c4614cdcf32 100644 --- a/src/util.h +++ b/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 input); v8::Maybe GetValidatedFd(Environment* env, v8::Local input); +v8::Maybe GetValidFileMode(Environment* env, + v8::Local input, + uv_fs_type type); } // namespace node