Skip to content

Commit fa5dae1

Browse files
tniessenRafaelGSS
authored andcommitted
permission: fix Uint8Array path traversal
Previous security patches addressed path traversal vulnerabilities for string and Buffer inputs, but ignored Uint8Array inputs. This commit fixes the existing logic to account for the latter. The previous implementation would silently ignore unexpected inputs, whereas this commit introduces an explicit assertion to prevent that unsafe behavior. PR-URL: nodejs-private/node-private#456 Reviewed-By: Rafael Gonzaga <[email protected]> CVE-ID: CVE-2023-39332
1 parent cd35275 commit fa5dae1

File tree

2 files changed

+19
-2
lines changed

2 files changed

+19
-2
lines changed

lib/internal/fs/utils.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const {
2222
Symbol,
2323
TypedArrayPrototypeAt,
2424
TypedArrayPrototypeIncludes,
25+
uncurryThis,
2526
} = primordials;
2627

2728
const permission = require('internal/process/permission');
@@ -709,13 +710,16 @@ const validatePath = hideStackFrames((path, propName = 'path') => {
709710
// See: https://github.com/nodejs/node/pull/44004#discussion_r930958420
710711
// The permission model needs the absolute path for the fs_permission
711712
const resolvePath = pathModule.resolve;
713+
const { isBuffer: BufferIsBuffer, from: BufferFrom } = Buffer;
714+
const BufferToString = uncurryThis(Buffer.prototype.toString);
712715
function possiblyTransformPath(path) {
713716
if (permission.isEnabled()) {
714717
if (typeof path === 'string') {
715718
return resolvePath(path);
716-
} else if (Buffer.isBuffer(path)) {
717-
return Buffer.from(resolvePath(path.toString()));
718719
}
720+
assert(isUint8Array(path));
721+
if (!BufferIsBuffer(path)) path = BufferFrom(path);
722+
return BufferFrom(resolvePath(BufferToString(path)));
719723
}
720724
return path;
721725
}

test/fixtures/permission/fs-traversal.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const allowedFolder = process.env.ALLOWEDFOLDER;
1515
const traversalPath = allowedFolder + '../file.md';
1616
const traversalFolderPath = allowedFolder + '../folder';
1717
const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
18+
const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath);
1819

1920
{
2021
assert.ok(process.permission.has('fs.read', allowedFolder));
@@ -83,6 +84,18 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
8384
}));
8485
}
8586

87+
{
88+
assert.throws(() => {
89+
fs.readFile(uint8ArrayTraversalPath, (error) => {
90+
assert.ifError(error);
91+
});
92+
}, common.expectsError({
93+
code: 'ERR_ACCESS_DENIED',
94+
permission: 'FileSystemRead',
95+
resource: resolve(traversalPath),
96+
}));
97+
}
98+
8699
{
87100
assert.ok(!process.permission.has('fs.read', traversalPath));
88101
assert.ok(!process.permission.has('fs.write', traversalPath));

0 commit comments

Comments
 (0)