Skip to content

Commit

Permalink
fs: validate fd for sync calls on c++
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed Dec 3, 2023
1 parent 2e458d9 commit 3ef54df
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 36 deletions.
16 changes: 2 additions & 14 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,6 @@ function close(fd, callback = defaultCloseCallback) {
* @returns {void}
*/
function closeSync(fd) {
fd = getValidatedFd(fd);

binding.close(fd);
}

Expand Down Expand Up @@ -709,8 +707,6 @@ ObjectDefineProperty(read, kCustomPromisifyArgsSymbol,
* @returns {number}
*/
function readSync(fd, buffer, offsetOrOptions, length, position) {
fd = getValidatedFd(fd);

validateBuffer(buffer);

let offset = offsetOrOptions;
Expand Down Expand Up @@ -798,7 +794,6 @@ ObjectDefineProperty(readv, kCustomPromisifyArgsSymbol,
* @returns {number}
*/
function readvSync(fd, buffers, position) {
fd = getValidatedFd(fd);
validateBufferArray(buffers);

const ctx = {};
Expand Down Expand Up @@ -899,7 +894,6 @@ ObjectDefineProperty(write, kCustomPromisifyArgsSymbol,
* @returns {number}
*/
function writeSync(fd, buffer, offsetOrOptions, length, position) {
fd = getValidatedFd(fd);
const ctx = {};
let result;

Expand Down Expand Up @@ -988,7 +982,6 @@ ObjectDefineProperty(writev, kCustomPromisifyArgsSymbol, {
* @returns {number}
*/
function writevSync(fd, buffers, position) {
fd = getValidatedFd(fd);
validateBufferArray(buffers);

if (buffers.length === 0) {
Expand Down Expand Up @@ -1127,7 +1120,6 @@ function ftruncate(fd, len = 0, callback) {
* @returns {void}
*/
function ftruncateSync(fd, len = 0) {
fd = getValidatedFd(fd);
validateInteger(len, 'len');
len = MathMax(0, len);
binding.ftruncate(fd, len);
Expand Down Expand Up @@ -1293,7 +1285,6 @@ function fdatasync(fd, callback) {
* @returns {void}
*/
function fdatasyncSync(fd) {
fd = getValidatedFd(fd);
binding.fdatasync(fd);
}

Expand All @@ -1318,7 +1309,6 @@ function fsync(fd, callback) {
* @returns {void}
*/
function fsyncSync(fd) {
fd = getValidatedFd(fd);
binding.fsync(fd);
}

Expand Down Expand Up @@ -1622,7 +1612,6 @@ function statfs(path, options = { bigint: false }, callback) {
* @returns {Stats | undefined}
*/
function fstatSync(fd, options = { bigint: false }) {
fd = getValidatedFd(fd);
const stats = binding.fstat(fd, options.bigint, undefined, false);
if (stats === undefined) {
return;
Expand Down Expand Up @@ -1905,7 +1894,7 @@ function fchmod(fd, mode, callback) {
*/
function fchmodSync(fd, mode) {
binding.fchmod(
getValidatedFd(fd),
fd,
parseFileMode(mode, 'mode'),
);
}
Expand Down Expand Up @@ -2051,7 +2040,6 @@ function fchown(fd, uid, gid, callback) {
* @returns {void}
*/
function fchownSync(fd, uid, gid) {
fd = getValidatedFd(fd);
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);

Expand Down Expand Up @@ -2168,7 +2156,7 @@ function futimes(fd, atime, mtime, callback) {
*/
function futimesSync(fd, atime, mtime) {
binding.futimes(
getValidatedFd(fd),
fd,
toUnixTimestamp(atime, 'atime'),
toUnixTimestamp(mtime, 'mtime'),
);
Expand Down
57 changes: 35 additions & 22 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 "util.h"

#include <fcntl.h>
#include <sys/types.h>
Expand Down Expand Up @@ -1405,13 +1406,14 @@ static void FTruncate(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 2);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 2;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(IsSafeJsInt(args[1]));
const int64_t len = args[1].As<Integer>()->Value();

if (argc > 2) {
if (is_async_call) {
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FTRUNCATE, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "ftruncate", UTF8, AfterNoArgs,
Expand All @@ -1430,10 +1432,11 @@ static void Fdatasync(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 1);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 1;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

if (argc > 1) { // fdatasync(fd, req)
if (is_async_call) { // fdatasync(fd, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 1);
CHECK_NOT_NULL(req_wrap_async);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FDATASYNC, req_wrap_async)
Expand All @@ -1453,10 +1456,11 @@ static void Fsync(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 1);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 1;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

if (argc > 1) {
if (is_async_call) {
FSReqBase* req_wrap_async = GetReqWrap(args, 1);
CHECK_NOT_NULL(req_wrap_async);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FSYNC, req_wrap_async)
Expand Down Expand Up @@ -2033,7 +2037,8 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 4);

CHECK(args[0]->IsInt32());
const bool is_async_call = !args[5]->IsUndefined();
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(Buffer::HasInstance(args[1]));
Expand Down Expand Up @@ -2088,7 +2093,8 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 3);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 3;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(args[1]->IsArray());
Expand All @@ -2104,7 +2110,7 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk));
}

if (argc > 3) { // writeBuffers(fd, chunks, pos, req)
if (is_async_call) { // writeBuffers(fd, chunks, pos, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
FS_ASYNC_TRACE_BEGIN0(UV_FS_WRITE, req_wrap_async)
AsyncCall(env,
Expand Down Expand Up @@ -2146,7 +2152,9 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {

const int argc = args.Length();
CHECK_GE(argc, 4);
CHECK(args[0]->IsInt32());

const bool is_async_call = !args[4]->IsUndefined();
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

const int64_t pos = GetOffset(args[2]);
Expand Down Expand Up @@ -2326,7 +2334,8 @@ static void Read(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 5);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 5;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(Buffer::HasInstance(args[1]));
Expand All @@ -2352,7 +2361,7 @@ static void Read(const FunctionCallbackInfo<Value>& args) {
char* buf = buffer_data + off;
uv_buf_t uvbuf = uv_buf_init(buf, len);

if (argc > 5) { // read(fd, buffer, offset, len, pos, req)
if (is_async_call) { // read(fd, buffer, offset, len, pos, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 5);
CHECK_NOT_NULL(req_wrap_async);
FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, req_wrap_async)
Expand Down Expand Up @@ -2450,7 +2459,8 @@ static void ReadBuffers(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 3);

CHECK(args[0]->IsInt32());
const bool is_async_call = !args[3]->IsUndefined();
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(args[1]->IsArray());
Expand All @@ -2467,8 +2477,8 @@ static void ReadBuffers(const FunctionCallbackInfo<Value>& args) {
iovs[i] = uv_buf_init(Buffer::Data(buffer), Buffer::Length(buffer));
}

FSReqBase* req_wrap_async = GetReqWrap(args, 3);
if (req_wrap_async != nullptr) { // readBuffers(fd, buffers, pos, req)
if (is_async_call) { // readBuffers(fd, buffers, pos, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "read", UTF8, AfterInteger,
uv_fs_read, fd, *iovs, iovs.length(), pos);
Expand Down Expand Up @@ -2525,13 +2535,14 @@ static void FChmod(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 2);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 2;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(args[1]->IsInt32());
const int mode = args[1].As<Int32>()->Value();

if (argc > 2) { // fchmod(fd, mode, req)
if (is_async_call) { // fchmod(fd, mode, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FCHMOD, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "fchmod", UTF8, AfterNoArgs,
Expand Down Expand Up @@ -2588,7 +2599,8 @@ static void FChown(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 3);

CHECK(args[0]->IsInt32());
const bool is_async_call = !args[3]->IsUndefined();
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(IsSafeJsInt(args[1]));
Expand All @@ -2597,8 +2609,8 @@ static void FChown(const FunctionCallbackInfo<Value>& args) {
CHECK(IsSafeJsInt(args[2]));
const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());

FSReqBase* req_wrap_async = GetReqWrap(args, 3);
if (req_wrap_async != nullptr) { // fchown(fd, uid, gid, req)
if (is_async_call) { // fchown(fd, uid, gid, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FCHOWN, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "fchown", UTF8, AfterNoArgs,
uv_fs_fchown, fd, uid, gid);
Expand Down Expand Up @@ -2683,7 +2695,8 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
const int argc = args.Length();
CHECK_GE(argc, 3);

CHECK(args[0]->IsInt32());
const bool is_async_call = argc > 3;
VALIDATE_FD(env, args[0], is_async_call);
const int fd = args[0].As<Int32>()->Value();

CHECK(args[1]->IsNumber());
Expand All @@ -2692,7 +2705,7 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
CHECK(args[2]->IsNumber());
const double mtime = args[2].As<Number>()->Value();

if (argc > 3) { // futimes(fd, atime, mtime, req)
if (is_async_call) { // futimes(fd, atime, mtime, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
FS_ASYNC_TRACE_BEGIN0(UV_FS_FUTIME, req_wrap_async)
AsyncCall(env, req_wrap_async, args, "futime", UTF8, AfterNoArgs,
Expand Down
28 changes: 28 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,34 @@ void DumpJavaScriptBacktrace(FILE* fp);
#define UNREACHABLE(...) \
ERROR_AND_ABORT("Unreachable code reached" __VA_OPT__(": ") __VA_ARGS__)

#define VALIDATE_FD(env, input, is_async_call) \
do { \
if (is_async_call) { \
CHECK(input->IsInt32()); \
return; \
} \
if (!input->IsInt32()) { \
Utf8Value value(env->isolate(), \
input->ToString(env->context()).ToLocalChecked()); \
Utf8Value type(env->isolate(), input->TypeOf(env->isolate())); \
THROW_ERR_INVALID_ARG_TYPE(env, \
"The \"fd\" argument must be of type " \
"number. Received type %s (%s)", \
type.out(), \
value.out()); \
return; \
} \
const int fd = input.As<Int32>()->Value(); \
if (fd < 0 || fd > INT32_MAX) { \
THROW_ERR_OUT_OF_RANGE(env, \
"The value of \"fd\" is out of range. " \
"It must be >= 0 && <= %s. Received %d", \
std::to_string(INT32_MAX), \
fd); \
return; \
} \
} while (0);

// ECMA262 20.1.2.6 Number.MAX_SAFE_INTEGER (2^53-1)
constexpr int64_t kMaxSafeJsInteger = 9007199254740991;

Expand Down

0 comments on commit 3ef54df

Please sign in to comment.