Skip to content

Commit

Permalink
fs: validate file mode from cpp
Browse files Browse the repository at this point in the history
PR-URL: nodejs#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>
  • Loading branch information
anonrig authored and rdw-msft committed Mar 26, 2024
1 parent c6c57d5 commit 73cd8a1
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 18 deletions.
7 changes: 1 addition & 6 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ const {
getOptions,
getValidatedFd,
getValidatedPath,
getValidMode,
handleErrorFromBinding,
preprocessSymlinkDestination,
Stats,
Expand Down Expand Up @@ -233,7 +232,6 @@ function access(path, mode, callback) {
}

path = getValidatedPath(path);
mode = getValidMode(mode, 'access');
callback = makeCallback(callback);

const req = new FSReqCallback();
Expand All @@ -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);
}

Expand Down Expand Up @@ -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();
Expand All @@ -3007,7 +3002,7 @@ function copyFileSync(src, dest, mode) {
binding.copyFile(
pathModule.toNamespacedPath(src),
pathModule.toNamespacedPath(dest),
getValidMode(mode, 'copyFile'),
mode,
);
}

Expand Down
3 changes: 0 additions & 3 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const {
getStatFsFromBinding,
getStatsFromBinding,
getValidatedPath,
getValidMode,
preprocessSymlinkDestination,
stringToFlags,
stringToSymlinkType,
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down
1 change: 0 additions & 1 deletion lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,6 @@ module.exports = {
getOptions,
getValidatedFd,
getValidatedPath,
getValidMode,
handleErrorFromBinding,
possiblyTransformPath,
preprocessSymlinkDestination,
Expand Down
19 changes: 12 additions & 7 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down
72 changes: 71 additions & 1 deletion src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "util.h" // NOLINT(build/include_inline)
#include <cmath>
#include <cstdint>
#include "util-inl.h"

#include "debug_utils-inl.h"
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include "uv.h"
#include "v8.h"

#include "node.h"
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 73cd8a1

Please sign in to comment.