Skip to content

Commit 301c45b

Browse files
committed
fs: validate fd for sync calls on c++
1 parent 2e458d9 commit 301c45b

File tree

8 files changed

+189
-56
lines changed

8 files changed

+189
-56
lines changed

lib/fs.js

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -513,13 +513,12 @@ function defaultCloseCallback(err) {
513513
* @returns {void}
514514
*/
515515
function close(fd, callback = defaultCloseCallback) {
516-
fd = getValidatedFd(fd);
517516
if (callback !== defaultCloseCallback)
518517
callback = makeCallback(callback);
519518

520519
const req = new FSReqCallback();
521520
req.oncomplete = callback;
522-
binding.close(fd, req);
521+
binding.close(getValidatedFd(fd), req);
523522
}
524523

525524
/**
@@ -528,9 +527,7 @@ function close(fd, callback = defaultCloseCallback) {
528527
* @returns {void}
529528
*/
530529
function closeSync(fd) {
531-
fd = getValidatedFd(fd);
532-
533-
binding.close(fd);
530+
binding.close(getValidatedFd(fd));
534531
}
535532

536533
/**
@@ -1110,7 +1107,6 @@ function ftruncate(fd, len = 0, callback) {
11101107
callback = len;
11111108
len = 0;
11121109
}
1113-
fd = getValidatedFd(fd);
11141110
validateInteger(len, 'len');
11151111
len = MathMax(0, len);
11161112
callback = makeCallback(callback);
@@ -1127,7 +1123,6 @@ function ftruncate(fd, len = 0, callback) {
11271123
* @returns {void}
11281124
*/
11291125
function ftruncateSync(fd, len = 0) {
1130-
fd = getValidatedFd(fd);
11311126
validateInteger(len, 'len');
11321127
len = MathMax(0, len);
11331128
binding.ftruncate(fd, len);
@@ -1279,7 +1274,6 @@ function rmSync(path, options) {
12791274
* @returns {void}
12801275
*/
12811276
function fdatasync(fd, callback) {
1282-
fd = getValidatedFd(fd);
12831277
const req = new FSReqCallback();
12841278
req.oncomplete = makeCallback(callback);
12851279
binding.fdatasync(fd, req);
@@ -1293,7 +1287,6 @@ function fdatasync(fd, callback) {
12931287
* @returns {void}
12941288
*/
12951289
function fdatasyncSync(fd) {
1296-
fd = getValidatedFd(fd);
12971290
binding.fdatasync(fd);
12981291
}
12991292

@@ -1305,7 +1298,6 @@ function fdatasyncSync(fd) {
13051298
* @returns {void}
13061299
*/
13071300
function fsync(fd, callback) {
1308-
fd = getValidatedFd(fd);
13091301
const req = new FSReqCallback();
13101302
req.oncomplete = makeCallback(callback);
13111303
binding.fsync(fd, req);
@@ -1318,7 +1310,6 @@ function fsync(fd, callback) {
13181310
* @returns {void}
13191311
*/
13201312
function fsyncSync(fd) {
1321-
fd = getValidatedFd(fd);
13221313
binding.fsync(fd);
13231314
}
13241315

@@ -1539,7 +1530,6 @@ function fstat(fd, options = { bigint: false }, callback) {
15391530
callback = options;
15401531
options = kEmptyObject;
15411532
}
1542-
fd = getValidatedFd(fd);
15431533
callback = makeStatsCallback(callback);
15441534

15451535
const req = new FSReqCallback(options.bigint);
@@ -1622,7 +1612,6 @@ function statfs(path, options = { bigint: false }, callback) {
16221612
* @returns {Stats | undefined}
16231613
*/
16241614
function fstatSync(fd, options = { bigint: false }) {
1625-
fd = getValidatedFd(fd);
16261615
const stats = binding.fstat(fd, options.bigint, undefined, false);
16271616
if (stats === undefined) {
16281617
return;
@@ -1888,7 +1877,6 @@ function unlinkSync(path) {
18881877
* @returns {void}
18891878
*/
18901879
function fchmod(fd, mode, callback) {
1891-
fd = getValidatedFd(fd);
18921880
mode = parseFileMode(mode, 'mode');
18931881
callback = makeCallback(callback);
18941882

@@ -1905,7 +1893,7 @@ function fchmod(fd, mode, callback) {
19051893
*/
19061894
function fchmodSync(fd, mode) {
19071895
binding.fchmod(
1908-
getValidatedFd(fd),
1896+
fd,
19091897
parseFileMode(mode, 'mode'),
19101898
);
19111899
}
@@ -2033,14 +2021,13 @@ function lchownSync(path, uid, gid) {
20332021
* @returns {void}
20342022
*/
20352023
function fchown(fd, uid, gid, callback) {
2036-
fd = getValidatedFd(fd);
20372024
validateInteger(uid, 'uid', -1, kMaxUserId);
20382025
validateInteger(gid, 'gid', -1, kMaxUserId);
20392026
callback = makeCallback(callback);
20402027

20412028
const req = new FSReqCallback();
20422029
req.oncomplete = callback;
2043-
binding.fchown(fd, uid, gid, req);
2030+
binding.fchown(getValidatedFd(fd), uid, gid, req);
20442031
}
20452032

20462033
/**
@@ -2051,12 +2038,11 @@ function fchown(fd, uid, gid, callback) {
20512038
* @returns {void}
20522039
*/
20532040
function fchownSync(fd, uid, gid) {
2054-
fd = getValidatedFd(fd);
20552041
validateInteger(uid, 'uid', -1, kMaxUserId);
20562042
validateInteger(gid, 'gid', -1, kMaxUserId);
20572043

20582044
const ctx = {};
2059-
binding.fchown(fd, uid, gid, undefined, ctx);
2045+
binding.fchown(getValidatedFd(fd), uid, gid, undefined, ctx);
20602046
handleErrorFromBinding(ctx);
20612047
}
20622048

@@ -2147,7 +2133,6 @@ function utimesSync(path, atime, mtime) {
21472133
* @returns {void}
21482134
*/
21492135
function futimes(fd, atime, mtime, callback) {
2150-
fd = getValidatedFd(fd);
21512136
atime = toUnixTimestamp(atime, 'atime');
21522137
mtime = toUnixTimestamp(mtime, 'mtime');
21532138
callback = makeCallback(callback);
@@ -2168,7 +2153,7 @@ function futimes(fd, atime, mtime, callback) {
21682153
*/
21692154
function futimesSync(fd, atime, mtime) {
21702155
binding.futimes(
2171-
getValidatedFd(fd),
2156+
fd,
21722157
toUnixTimestamp(atime, 'atime'),
21732158
toUnixTimestamp(mtime, 'mtime'),
21742159
);

src/node_file.cc

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,15 @@
3838
#include "req_wrap-inl.h"
3939
#include "stream_base-inl.h"
4040
#include "string_bytes.h"
41+
#include "util.h"
4142

4243
#include <fcntl.h>
43-
#include <sys/types.h>
4444
#include <sys/stat.h>
45-
#include <cstring>
45+
#include <sys/types.h>
4646
#include <cerrno>
4747
#include <climits>
48+
#include <cstdio>
49+
#include <cstring>
4850

4951
#if defined(__MINGW32__) || defined(_MSC_VER)
5052
# include <io.h>
@@ -1154,7 +1156,7 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
11541156
const int argc = args.Length();
11551157
CHECK_GE(argc, 2);
11561158

1157-
CHECK(args[0]->IsInt32());
1159+
VALIDATE_FD(env, args[0], false);
11581160
int fd = args[0].As<Int32>()->Value();
11591161

11601162
bool use_bigint = args[1]->IsTrue();
@@ -1405,18 +1407,18 @@ static void FTruncate(const FunctionCallbackInfo<Value>& args) {
14051407
const int argc = args.Length();
14061408
CHECK_GE(argc, 2);
14071409

1408-
CHECK(args[0]->IsInt32());
1410+
VALIDATE_FD(env, args[0], false);
14091411
const int fd = args[0].As<Int32>()->Value();
14101412

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

1414-
if (argc > 2) {
1416+
if (argc > 2) { // ftruncate(fd, len, req)
14151417
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
14161418
FS_ASYNC_TRACE_BEGIN0(UV_FS_FTRUNCATE, req_wrap_async)
14171419
AsyncCall(env, req_wrap_async, args, "ftruncate", UTF8, AfterNoArgs,
14181420
uv_fs_ftruncate, fd, len);
1419-
} else {
1421+
} else { // ftruncate(fd, len)
14201422
FSReqWrapSync req_wrap_sync("ftruncate");
14211423
FS_SYNC_TRACE_BEGIN(ftruncate);
14221424
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_ftruncate, fd, len);
@@ -1430,7 +1432,7 @@ static void Fdatasync(const FunctionCallbackInfo<Value>& args) {
14301432
const int argc = args.Length();
14311433
CHECK_GE(argc, 1);
14321434

1433-
CHECK(args[0]->IsInt32());
1435+
VALIDATE_FD(env, args[0], false);
14341436
const int fd = args[0].As<Int32>()->Value();
14351437

14361438
if (argc > 1) { // fdatasync(fd, req)
@@ -1453,7 +1455,7 @@ static void Fsync(const FunctionCallbackInfo<Value>& args) {
14531455
const int argc = args.Length();
14541456
CHECK_GE(argc, 1);
14551457

1456-
CHECK(args[0]->IsInt32());
1458+
VALIDATE_FD(env, args[0], false);
14571459
const int fd = args[0].As<Int32>()->Value();
14581460

14591461
if (argc > 1) {
@@ -2525,8 +2527,13 @@ static void FChmod(const FunctionCallbackInfo<Value>& args) {
25252527
const int argc = args.Length();
25262528
CHECK_GE(argc, 2);
25272529

2528-
CHECK(args[0]->IsInt32());
2530+
VALIDATE_FD(env, args[0], false);
2531+
auto t = args[0]->TypeOf(env->isolate());
2532+
Utf8Value type(env->isolate(), t);
2533+
printf("type of args[0] is %s\n", type.out());
2534+
printf("int32max %s\n", std::to_string(INT32_MAX).c_str());
25292535
const int fd = args[0].As<Int32>()->Value();
2536+
printf("fd is %d\n", fd);
25302537

25312538
CHECK(args[1]->IsInt32());
25322539
const int mode = args[1].As<Int32>()->Value();
@@ -2683,7 +2690,7 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
26832690
const int argc = args.Length();
26842691
CHECK_GE(argc, 3);
26852692

2686-
CHECK(args[0]->IsInt32());
2693+
VALIDATE_FD(env, args[0], false);
26872694
const int fd = args[0].As<Int32>()->Value();
26882695

26892696
CHECK(args[1]->IsNumber());

src/util.cc

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

2222
#include "util.h" // NOLINT(build/include_inline)
23+
#include <cmath>
24+
#include "debug_utils.h"
2325
#include "util-inl.h"
2426

2527
#include "debug_utils-inl.h"
@@ -31,6 +33,7 @@
3133
#include "node_v8_platform-inl.h"
3234
#include "string_bytes.h"
3335
#include "uv.h"
36+
#include "v8-value.h"
3437

3538
#ifdef _WIN32
3639
#include <io.h> // _S_IREAD _S_IWRITE
@@ -702,4 +705,51 @@ RAIIIsolate::RAIIIsolate(const SnapshotData* data)
702705

703706
RAIIIsolate::~RAIIIsolate() {}
704707

708+
// Returns a string representation of the input value, including type.
709+
// JavaScript implementation is available in lib/internal/errors.js
710+
std::string DetermineSpecificErrorType(Environment* env,
711+
v8::Local<v8::Value> input) {
712+
if (input->IsFunction()) {
713+
return "function";
714+
} else if (input->IsString()) {
715+
auto value = Utf8Value(env->isolate(), input).ToString();
716+
if (value.size() > 28) {
717+
value = value.substr(0, 25) + "...";
718+
}
719+
if (value.find('\'') == std::string::npos) {
720+
return SPrintF("type string ('%s')", value);
721+
}
722+
723+
// Stringify the input value.
724+
Local<String> stringified =
725+
v8::JSON::Stringify(env->context(), input).ToLocalChecked();
726+
Utf8Value stringified_value(env->isolate(), stringified);
727+
return SPrintF("type string (%s)", stringified_value.out());
728+
} else if (input->IsObject()) {
729+
v8::Local<v8::String> constructor_name =
730+
input.As<v8::Object>()->GetConstructorName();
731+
Utf8Value name(env->isolate(), constructor_name);
732+
return SPrintF("an instance of %s", name.out());
733+
}
734+
735+
Utf8Value utf8_value(env->isolate(),
736+
input->ToString(env->context()).ToLocalChecked());
737+
738+
if (input->IsNumber() || input->IsInt32() || input->IsUint32()) {
739+
auto value = input.As<v8::Number>()->Value();
740+
if (std::isnan(value)) {
741+
return "type number (NaN)";
742+
} else if (std::isinf(value)) {
743+
return "type number (Infinity)";
744+
}
745+
return SPrintF("type number (%s)", utf8_value.out());
746+
} else if (input->IsBigInt() || input->IsBoolean() || input->IsSymbol()) {
747+
Utf8Value type(env->isolate(), input->TypeOf(env->isolate()));
748+
return SPrintF("type %s (%s)", type.out(), utf8_value.out());
749+
}
750+
751+
// For example: null, undefined
752+
return utf8_value.ToString();
753+
}
754+
705755
} // namespace node

src/util.h

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,42 @@ void DumpJavaScriptBacktrace(FILE* fp);
195195
#define UNREACHABLE(...) \
196196
ERROR_AND_ABORT("Unreachable code reached" __VA_OPT__(": ") __VA_ARGS__)
197197

198+
#define VALIDATE_FD(env, input, has_req) \
199+
do { \
200+
if (has_req) { \
201+
CHECK(input->IsInt32()); \
202+
return; \
203+
} \
204+
if (!input->IsInt32() && !input->IsNumber()) { \
205+
std::string error_type = node::DetermineSpecificErrorType(env, input); \
206+
THROW_ERR_INVALID_ARG_TYPE(env, \
207+
"The \"fd\" argument must be of type " \
208+
"number. Received %s", \
209+
error_type.c_str()); \
210+
return; \
211+
} \
212+
const auto fd = input.As<Number>()->Value(); \
213+
const bool is_out_of_range = fd < 0 || fd > INT32_MAX; \
214+
if (is_out_of_range || !IsSafeJsInt(input)) { \
215+
Utf8Value utf8_value(env->isolate(), \
216+
input->ToString(env->context()).ToLocalChecked()); \
217+
if (is_out_of_range && !std::isinf(fd)) { \
218+
THROW_ERR_OUT_OF_RANGE(env, \
219+
"The value of \"fd\" is out of range. " \
220+
"It must be >= 0 && <= %s. Received %d", \
221+
std::to_string(INT32_MAX), \
222+
utf8_value.out()); \
223+
} else { \
224+
THROW_ERR_OUT_OF_RANGE( \
225+
env, \
226+
"The value of \"fd\" is out of range. It must be an integer. " \
227+
"Received %s", \
228+
utf8_value.out()); \
229+
} \
230+
return; \
231+
} \
232+
} while (0);
233+
198234
// ECMA262 20.1.2.6 Number.MAX_SAFE_INTEGER (2^53-1)
199235
constexpr int64_t kMaxSafeJsInteger = 9007199254740991;
200236

@@ -996,6 +1032,9 @@ class RAIIIsolate {
9961032
v8::Isolate::Scope isolate_scope_;
9971033
};
9981034

1035+
std::string DetermineSpecificErrorType(Environment* env,
1036+
v8::Local<v8::Value> input);
1037+
9991038
} // namespace node
10001039

10011040
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

0 commit comments

Comments
 (0)