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: https://github.com/nodejs-private/node-private/pull/456 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> CVE-ID: CVE-2023-39332
This commit is contained in:
parent
32bcf4ca27
commit
f447a4611a
@ -22,6 +22,7 @@ const {
|
||||
Symbol,
|
||||
TypedArrayPrototypeAt,
|
||||
TypedArrayPrototypeIncludes,
|
||||
uncurryThis,
|
||||
} = primordials;
|
||||
|
||||
const permission = require('internal/process/permission');
|
||||
@ -709,13 +710,16 @@ const validatePath = hideStackFrames((path, propName = 'path') => {
|
||||
// See: https://github.com/nodejs/node/pull/44004#discussion_r930958420
|
||||
// The permission model needs the absolute path for the fs_permission
|
||||
const resolvePath = pathModule.resolve;
|
||||
const { isBuffer: BufferIsBuffer, from: BufferFrom } = Buffer;
|
||||
const BufferToString = uncurryThis(Buffer.prototype.toString);
|
||||
function possiblyTransformPath(path) {
|
||||
if (permission.isEnabled()) {
|
||||
if (typeof path === 'string') {
|
||||
return resolvePath(path);
|
||||
} else if (Buffer.isBuffer(path)) {
|
||||
return Buffer.from(resolvePath(path.toString()));
|
||||
}
|
||||
assert(isUint8Array(path));
|
||||
if (!BufferIsBuffer(path)) path = BufferFrom(path);
|
||||
return BufferFrom(resolvePath(BufferToString(path)));
|
||||
}
|
||||
return path;
|
||||
}
|
||||
|
13
test/fixtures/permission/fs-traversal.js
vendored
13
test/fixtures/permission/fs-traversal.js
vendored
@ -15,6 +15,7 @@ const allowedFolder = process.env.ALLOWEDFOLDER;
|
||||
const traversalPath = allowedFolder + '../file.md';
|
||||
const traversalFolderPath = allowedFolder + '../folder';
|
||||
const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
|
||||
const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath);
|
||||
|
||||
{
|
||||
assert.ok(process.permission.has('fs.read', allowedFolder));
|
||||
@ -83,6 +84,18 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
|
||||
}));
|
||||
}
|
||||
|
||||
{
|
||||
assert.throws(() => {
|
||||
fs.readFile(uint8ArrayTraversalPath, (error) => {
|
||||
assert.ifError(error);
|
||||
});
|
||||
}, common.expectsError({
|
||||
code: 'ERR_ACCESS_DENIED',
|
||||
permission: 'FileSystemRead',
|
||||
resource: resolve(traversalPath),
|
||||
}));
|
||||
}
|
||||
|
||||
{
|
||||
assert.ok(!process.permission.has('fs.read', traversalPath));
|
||||
assert.ok(!process.permission.has('fs.write', traversalPath));
|
||||
|
Loading…
x
Reference in New Issue
Block a user