1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

permission: improve path traversal protection

Browse Source 
Always use the original implementation of pathModule.resolve. If the
application overwrites the value of pathModule.resolve with a custom
implementation, it should not have any effect on the permission model.

PR-URL: https://github.com/nodejs-private/node-private/pull/456
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2023-39331
This commit is contained in:
Tobias Nießen 2023-08-01 22:36:12 +00:00 committed by RafaelGSS
parent 3b23b2ee53
commit 32bcf4ca27
2 changed files with 12 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/internal/fs/utils.js
View File

@ -708,12 +708,13 @@ const validatePath = hideStackFrames((path, propName = 'path') => {
// TODO(rafaelgss): implement the path.resolve on C++ side
// 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;
function possiblyTransformPath(path) {
if (permission.isEnabled()) {
if (typeof path === 'string') {
return pathModule.resolve(path);
return resolvePath(path);
} else if (Buffer.isBuffer(path)) {
return Buffer.from(pathModule.resolve(path.toString()));
return Buffer.from(resolvePath(path.toString()));
}
}
return path;

14
test/fixtures/permission/fs-traversal.js vendored
View File

@ -6,6 +6,10 @@ const assert = require('assert');
const fs = require('fs');
const path = require('path');
// This should not affect how the permission model resolves paths.
const { resolve } = path;
path.resolve = (s) => s;
const blockedFolder = process.env.BLOCKEDFOLDER;
const allowedFolder = process.env.ALLOWEDFOLDER;
const traversalPath = allowedFolder + '../file.md';
@ -27,7 +31,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
resource: path.toNamespacedPath(path.resolve(traversalPath)),
resource: path.toNamespacedPath(resolve(traversalPath)),
}));
}
@ -39,7 +43,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
resource: path.toNamespacedPath(path.resolve(traversalPath)),
resource: path.toNamespacedPath(resolve(traversalPath)),
}));
}
@ -51,7 +55,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
resource: path.resolve(traversalFolderPath + 'XXXXXX'),
resource: resolve(traversalFolderPath + 'XXXXXX'),
}));
}
@ -63,7 +67,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
resource: path.resolve(traversalFolderPath + 'XXXXXX'),
resource: resolve(traversalFolderPath + 'XXXXXX'),
}));
}
@ -75,7 +79,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
resource: path.resolve(traversalPath),
resource: resolve(traversalPath),
}));
}