Nodejs/node
1
0
Fork 0
mirror of https://github.com/nodejs/node.git synced 2024-11-21 10:59:27 +00:00
Code Issues Actions 25 Packages Projects Releases Wiki Activity

fs: harden fs.readSync(buffer, options) typecheck

Browse Source 
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:
LiviaMedeiros 2022-04-19 14:19:55 +08:00 committed by Node.js GitHub Bot
parent 2275faac2b
commit 41a6d82968
2 changed files with 34 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
lib/fs.js
View File

@ -701,26 +701,28 @@ ObjectDefineProperty(read, kCustomPromisifyArgsSymbol,
* offset?: number;
* length?: number;
* position?: number | bigint | null;
* }} [offset]
* }} [offsetOrOptions]
* @returns {number}
*/
function readSync(fd, buffer, offset, length, position) {
function readSync(fd, buffer, offsetOrOptions, length, position) {
fd = getValidatedFd(fd);
validateBuffer(buffer);
if (arguments.length <= 3) {
// Assume fs.readSync(fd, buffer, options)
const options = offset || kEmptyObject;
let offset = offsetOrOptions;
if (arguments.length <= 3 || typeof offsetOrOptions === 'object') {
if (offsetOrOptions !== undefined) {
validateObject(offsetOrOptions, 'options', { nullable: true });
}
({
offset = 0,
length = buffer.byteLength - offset,
position = null,
} = options);
} = offsetOrOptions ?? kEmptyObject);
}
if (offset == null) {
if (offset === undefined) {
offset = 0;
} else {
validateInteger(offset, 'offset', 0);

34
test/parallel/test-fs-readSync-optional-params.js
View File

@ -8,13 +8,20 @@ const filepath = fixtures.path('x.txt');
const expected = Buffer.from('xyz\n');
function runTest(defaultBuffer, options) {
function runTest(defaultBuffer, options, errorCode = false) {
let fd;
try {
fd = fs.openSync(filepath, 'r');
const result = fs.readSync(fd, defaultBuffer, options);
assert.strictEqual(result, expected.length);
assert.deepStrictEqual(defaultBuffer, expected);
if (errorCode) {
assert.throws(
() => 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 {
if (fd != null) fs.closeSync(fd);
}
@ -31,7 +38,6 @@ for (const options of [
{ length: expected.length, position: 0 },
{ offset: 0, length: expected.length, position: 0 },
{ offset: null },
{ position: null },
{ position: -1 },
{ position: 0n },
@ -41,17 +47,27 @@ for (const options of [
null,
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,
true,
Infinity,
42n,
Symbol(),
'amString',
[],
() => {},
// Test even more malicious corner cases
// Test if arbitrary entity with expected .length is not mistaken for options
'4'.repeat(expected.length),
new String('4444'),
[4, 4, 4, 4],
]) {
runTest(Buffer.allocUnsafe(expected.length), mustNotMutateObjectDeep(options));
runTest(Buffer.allocUnsafe(expected.length), mustNotMutateObjectDeep(options), 'ERR_INVALID_ARG_TYPE');
}