Skip to content

Commit ed7d149

Browse files
committed
lib: use cache fs internals against path traversal
PR-URL: nodejs-private/node-private#516 Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=2259914 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> CVE-ID: CVE-2024-21891
1 parent 480fc16 commit ed7d149

File tree

2 files changed

+28
-2
lines changed

2 files changed

+28
-2
lines changed

lib/internal/process/pre_execution.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
NumberParseInt,
1313
ObjectDefineProperties,
1414
ObjectDefineProperty,
15+
ObjectFreeze,
1516
ObjectGetOwnPropertyDescriptor,
1617
SafeMap,
1718
String,
@@ -597,6 +598,8 @@ function initializePermission() {
597598
process.binding = function binding(_module) {
598599
throw new ERR_ACCESS_DENIED('process.binding');
599600
};
601+
// Guarantee path module isn't monkey-patched to bypass permission model
602+
ObjectFreeze(require('path'));
600603
process.emitWarning('Permission is an experimental feature',
601604
'ExperimentalWarning');
602605
const { has, deny } = require('internal/process/permission');

test/fixtures/permission/fs-traversal.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@ const assert = require('assert');
66
const fs = require('fs');
77
const path = require('path');
88

9-
// This should not affect how the permission model resolves paths.
109
const { resolve } = path;
11-
path.resolve = (s) => s;
10+
// This should not affect how the permission model resolves paths.
11+
try {
12+
path.resolve = (s) => s;
13+
assert.fail('should not be called');
14+
} catch {}
1215

1316
const blockedFolder = process.env.BLOCKEDFOLDER;
1417
const allowedFolder = process.env.ALLOWEDFOLDER;
@@ -96,6 +99,26 @@ const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath);
9699
}));
97100
}
98101

102+
// Monkey-patching path module should also not allow path traversal.
103+
{
104+
const fs = require('fs');
105+
const path = require('path');
106+
107+
const cwd = Buffer.from('.');
108+
try {
109+
path.toNamespacedPath = (path) => { return traversalPath; };
110+
assert.fail('should throw error when pacthing');
111+
} catch { }
112+
113+
assert.throws(() => {
114+
fs.readFile(cwd, common.mustNotCall());
115+
}, common.expectsError({
116+
code: 'ERR_ACCESS_DENIED',
117+
permission: 'FileSystemRead',
118+
resource: resolve(cwd.toString()),
119+
}));
120+
}
121+
99122
// Monkey-patching Buffer internals should also not allow path traversal.
100123
{
101124
const extraChars = '.'.repeat(40);

0 commit comments

Comments
 (0)