mirror of https://github.com/nodejs/node.git
fs: harden fs.readSync(buffer, options) typecheck
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: https://github.com/nodejs/node/pull/42772 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
parent
2275faac2b
commit
41a6d82968
16
lib/fs.js
16
lib/fs.js
|
@ -701,26 +701,28 @@ ObjectDefineProperty(read, kCustomPromisifyArgsSymbol,
|
||||||
* offset?: number;
|
* offset?: number;
|
||||||
* length?: number;
|
* length?: number;
|
||||||
* position?: number | bigint | null;
|
* position?: number | bigint | null;
|
||||||
* }} [offset]
|
* }} [offsetOrOptions]
|
||||||
* @returns {number}
|
* @returns {number}
|
||||||
*/
|
*/
|
||||||
function readSync(fd, buffer, offset, length, position) {
|
function readSync(fd, buffer, offsetOrOptions, length, position) {
|
||||||
fd = getValidatedFd(fd);
|
fd = getValidatedFd(fd);
|
||||||
|
|
||||||
validateBuffer(buffer);
|
validateBuffer(buffer);
|
||||||
|
|
||||||
if (arguments.length <= 3) {
|
let offset = offsetOrOptions;
|
||||||
// Assume fs.readSync(fd, buffer, options)
|
if (arguments.length <= 3 || typeof offsetOrOptions === 'object') {
|
||||||
const options = offset || kEmptyObject;
|
if (offsetOrOptions !== undefined) {
|
||||||
|
validateObject(offsetOrOptions, 'options', { nullable: true });
|
||||||
|
}
|
||||||
|
|
||||||
({
|
({
|
||||||
offset = 0,
|
offset = 0,
|
||||||
length = buffer.byteLength - offset,
|
length = buffer.byteLength - offset,
|
||||||
position = null,
|
position = null,
|
||||||
} = options);
|
} = offsetOrOptions ?? kEmptyObject);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (offset == null) {
|
if (offset === undefined) {
|
||||||
offset = 0;
|
offset = 0;
|
||||||
} else {
|
} else {
|
||||||
validateInteger(offset, 'offset', 0);
|
validateInteger(offset, 'offset', 0);
|
||||||
|
|
|
@ -8,13 +8,20 @@ const filepath = fixtures.path('x.txt');
|
||||||
|
|
||||||
const expected = Buffer.from('xyz\n');
|
const expected = Buffer.from('xyz\n');
|
||||||
|
|
||||||
function runTest(defaultBuffer, options) {
|
function runTest(defaultBuffer, options, errorCode = false) {
|
||||||
let fd;
|
let fd;
|
||||||
try {
|
try {
|
||||||
fd = fs.openSync(filepath, 'r');
|
fd = fs.openSync(filepath, 'r');
|
||||||
const result = fs.readSync(fd, defaultBuffer, options);
|
if (errorCode) {
|
||||||
assert.strictEqual(result, expected.length);
|
assert.throws(
|
||||||
assert.deepStrictEqual(defaultBuffer, expected);
|
() => fs.readSync(fd, defaultBuffer, options),
|
||||||
|
{ code: errorCode }
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
const result = fs.readSync(fd, defaultBuffer, options);
|
||||||
|
assert.strictEqual(result, expected.length);
|
||||||
|
assert.deepStrictEqual(defaultBuffer, expected);
|
||||||
|
}
|
||||||
} finally {
|
} finally {
|
||||||
if (fd != null) fs.closeSync(fd);
|
if (fd != null) fs.closeSync(fd);
|
||||||
}
|
}
|
||||||
|
@ -31,7 +38,6 @@ for (const options of [
|
||||||
{ length: expected.length, position: 0 },
|
{ length: expected.length, position: 0 },
|
||||||
{ offset: 0, length: expected.length, position: 0 },
|
{ offset: 0, length: expected.length, position: 0 },
|
||||||
|
|
||||||
{ offset: null },
|
|
||||||
{ position: null },
|
{ position: null },
|
||||||
{ position: -1 },
|
{ position: -1 },
|
||||||
{ position: 0n },
|
{ position: 0n },
|
||||||
|
@ -41,17 +47,27 @@ for (const options of [
|
||||||
null,
|
null,
|
||||||
undefined,
|
undefined,
|
||||||
|
|
||||||
// Test if bad params are interpreted as default (not mandatory)
|
// Test malicious corner case: it works as {length: 4} but not intentionally
|
||||||
|
new String('4444'),
|
||||||
|
]) {
|
||||||
|
runTest(Buffer.allocUnsafe(expected.length), options);
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const options of [
|
||||||
|
|
||||||
|
// Test various invalid options
|
||||||
false,
|
false,
|
||||||
true,
|
true,
|
||||||
Infinity,
|
Infinity,
|
||||||
42n,
|
42n,
|
||||||
Symbol(),
|
Symbol(),
|
||||||
|
'amString',
|
||||||
|
[],
|
||||||
|
() => {},
|
||||||
|
|
||||||
// Test even more malicious corner cases
|
// Test if arbitrary entity with expected .length is not mistaken for options
|
||||||
'4'.repeat(expected.length),
|
'4'.repeat(expected.length),
|
||||||
new String('4444'),
|
|
||||||
[4, 4, 4, 4],
|
[4, 4, 4, 4],
|
||||||
]) {
|
]) {
|
||||||
runTest(Buffer.allocUnsafe(expected.length), mustNotMutateObjectDeep(options));
|
runTest(Buffer.allocUnsafe(expected.length), mustNotMutateObjectDeep(options), 'ERR_INVALID_ARG_TYPE');
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue