Skip to content

Commit

Permalink
permission: fix Uint8Array path traversal
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tniessen authored and RafaelGSS committed Oct 13, 2023
1 parent 32bcf4c commit f447a46
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const {
Symbol,
TypedArrayPrototypeAt,
TypedArrayPrototypeIncludes,
uncurryThis,
} = primordials;

const permission = require('internal/process/permission');
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions test/fixtures/permission/fs-traversal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit f447a46

Please sign in to comment.