Skip to content

Commit

Permalink
lib: use cache fs internals against path traversal
Browse files Browse the repository at this point in the history
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
  • Loading branch information
RafaelGSS committed Feb 13, 2024
1 parent 480fc16 commit ed7d149
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
3 changes: 3 additions & 0 deletions lib/internal/process/pre_execution.js
Expand Up @@ -12,6 +12,7 @@ const {
NumberParseInt,
ObjectDefineProperties,
ObjectDefineProperty,
ObjectFreeze,
ObjectGetOwnPropertyDescriptor,
SafeMap,
String,
Expand Down Expand Up @@ -597,6 +598,8 @@ function initializePermission() {
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
// Guarantee path module isn't monkey-patched to bypass permission model
ObjectFreeze(require('path'));
process.emitWarning('Permission is an experimental feature',
'ExperimentalWarning');
const { has, deny } = require('internal/process/permission');
Expand Down
27 changes: 25 additions & 2 deletions test/fixtures/permission/fs-traversal.js
Expand Up @@ -6,9 +6,12 @@ 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;
// This should not affect how the permission model resolves paths.
try {
path.resolve = (s) => s;
assert.fail('should not be called');
} catch {}

const blockedFolder = process.env.BLOCKEDFOLDER;
const allowedFolder = process.env.ALLOWEDFOLDER;
Expand Down Expand Up @@ -96,6 +99,26 @@ const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath);
}));
}

// Monkey-patching path module should also not allow path traversal.
{
const fs = require('fs');
const path = require('path');

const cwd = Buffer.from('.');
try {
path.toNamespacedPath = (path) => { return traversalPath; };
assert.fail('should throw error when pacthing');
} catch { }

assert.throws(() => {
fs.readFile(cwd, common.mustNotCall());
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
resource: resolve(cwd.toString()),
}));
}

// Monkey-patching Buffer internals should also not allow path traversal.
{
const extraChars = '.'.repeat(40);
Expand Down

0 comments on commit ed7d149

Please sign in to comment.