Skip to content

Commit 355e305

Browse files
committed
fs: validate mode in CreateReadStream and CreateWriteStream
Large mode values (e.g. 2176057344) that exceed the Int32 range but fit within UInt32 cause a C++ assertion failure (args[2]->IsInt32()) instead of throwing a RangeError. This is a regression introduced by nodejs#52050 which moved mode validation to C++. Fix by: 1. Changing parseFileMode() to use validateInt32 instead of validateUint32 to match the C++ Int32 expectation. 2. Adding parseFileMode() calls in ReadStream/WriteStream constructors. Fixes: nodejs#62516
1 parent 0d7e4b1 commit 355e305

3 files changed

Lines changed: 41 additions & 3 deletions

File tree

lib/internal/fs/streams.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323
kEmptyObject,
2424
} = require('internal/util');
2525
const {
26+
parseFileMode,
2627
validateBoolean,
2728
validateFunction,
2829
validateInteger,
@@ -181,7 +182,7 @@ function ReadStream(path, options) {
181182
// Path will be ignored when fd is specified, so it can be falsy
182183
this.path = toPathIfFileURL(path);
183184
this.flags = options.flags === undefined ? 'r' : options.flags;
184-
this.mode = options.mode === undefined ? 0o666 : options.mode;
185+
this.mode = parseFileMode(options.mode, 'options.mode', 0o666);
185186

186187
validatePath(this.path);
187188
} else {
@@ -333,7 +334,7 @@ function WriteStream(path, options) {
333334
// Path will be ignored when fd is specified, so it can be falsy
334335
this.path = toPathIfFileURL(path);
335336
this.flags = options.flags === undefined ? 'w' : options.flags;
336-
this.mode = options.mode === undefined ? 0o666 : options.mode;
337+
this.mode = parseFileMode(options.mode, 'options.mode', 0o666);
337338

338339
validatePath(this.path);
339340
} else {

lib/internal/validators.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ function parseFileMode(value, name, def) {
7878
value = NumberParseInt(value, 8);
7979
}
8080

81-
validateUint32(value, name);
81+
validateInt32(value, name, 0);
8282
return value;
8383
}
8484

test/parallel/test-file-validate-mode-flag.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
'use strict';
22

33
// Checks for crash regression: https://github.com/nodejs/node/issues/37430
4+
// and https://github.com/nodejs/node/issues/62516
45

56
const common = require('../common');
67
const assert = require('assert');
78
const {
9+
createReadStream,
10+
createWriteStream,
811
open,
912
openSync,
1013
promises: {
@@ -15,6 +18,10 @@ const {
1518
// These should throw, not crash.
1619
const invalid = 4_294_967_296;
1720

21+
// Value within UInt32 range but exceeding Int32 range.
22+
// Regression test for https://github.com/nodejs/node/issues/62516
23+
const invalidInt32 = 2_176_057_344;
24+
1825
assert.throws(() => open(__filename, invalid, common.mustNotCall()), {
1926
code: 'ERR_OUT_OF_RANGE'
2027
});
@@ -23,6 +30,10 @@ assert.throws(() => open(__filename, 0, invalid, common.mustNotCall()), {
2330
code: 'ERR_OUT_OF_RANGE'
2431
});
2532

33+
assert.throws(() => open(__filename, 0, invalidInt32, common.mustNotCall()), {
34+
code: 'ERR_OUT_OF_RANGE'
35+
});
36+
2637
assert.throws(() => openSync(__filename, invalid), {
2738
code: 'ERR_OUT_OF_RANGE'
2839
});
@@ -31,10 +42,36 @@ assert.throws(() => openSync(__filename, 0, invalid), {
3142
code: 'ERR_OUT_OF_RANGE'
3243
});
3344

45+
assert.throws(() => openSync(__filename, 0, invalidInt32), {
46+
code: 'ERR_OUT_OF_RANGE'
47+
});
48+
3449
assert.rejects(openPromise(__filename, invalid), {
3550
code: 'ERR_OUT_OF_RANGE'
3651
}).then(common.mustCall());
3752

3853
assert.rejects(openPromise(__filename, 0, invalid), {
3954
code: 'ERR_OUT_OF_RANGE'
4055
}).then(common.mustCall());
56+
57+
assert.rejects(openPromise(__filename, 0, invalidInt32), {
58+
code: 'ERR_OUT_OF_RANGE'
59+
}).then(common.mustCall());
60+
61+
// createReadStream and createWriteStream should also validate mode.
62+
// Regression test for https://github.com/nodejs/node/issues/62516
63+
assert.throws(() => createReadStream(__filename, { mode: invalidInt32 }), {
64+
code: 'ERR_OUT_OF_RANGE'
65+
});
66+
67+
assert.throws(() => createWriteStream(__filename, { mode: invalidInt32 }), {
68+
code: 'ERR_OUT_OF_RANGE'
69+
});
70+
71+
assert.throws(() => createReadStream(__filename, { mode: invalid }), {
72+
code: 'ERR_OUT_OF_RANGE'
73+
});
74+
75+
assert.throws(() => createWriteStream(__filename, { mode: invalid }), {
76+
code: 'ERR_OUT_OF_RANGE'
77+
});

0 commit comments

Comments
 (0)