From ed7d14967599a9dbace7cabbe287d7c9d5b767a0 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Mon, 8 Jan 2024 16:03:43 -0300 Subject: [PATCH] lib: use cache fs internals against path traversal PR-URL: https://github.com/nodejs-private/node-private/pull/516 Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=2259914 Reviewed-By: Moshe Atlow CVE-ID: CVE-2024-21891 --- lib/internal/process/pre_execution.js | 3 +++ test/fixtures/permission/fs-traversal.js | 27 ++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 9142fed75e9050..cffffc28d3d703 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -12,6 +12,7 @@ const { NumberParseInt, ObjectDefineProperties, ObjectDefineProperty, + ObjectFreeze, ObjectGetOwnPropertyDescriptor, SafeMap, String, @@ -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'); diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js index f0a14805aa9c71..5a127f75474da8 100644 --- a/test/fixtures/permission/fs-traversal.js +++ b/test/fixtures/permission/fs-traversal.js @@ -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; @@ -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);