diff --git a/lib/fs.js b/lib/fs.js index 4a03fada49ea8a..b0da37c59db473 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -149,6 +149,10 @@ const { const permission = require('internal/process/permission'); +// Matches a dotdot path component (e.g. `..`, `/../`, `../`) but not +// filenames with consecutive dots like `a..b`. +const dotdotRe = /(^|[\\/])\.\.(?:[\\/]|$)/; + let fs; // Lazy loaded @@ -1179,6 +1183,14 @@ function rm(path, options, callback) { options = undefined; } path = getValidatedPath(path); + if (typeof path === 'string') { + if (SideEffectFreeRegExpPrototypeExec(dotdotRe, path) !== null) + path = pathModule.normalize(path); + } else if (isArrayBufferView(path)) { + const str = BufferToString(path, 'utf8'); + if (SideEffectFreeRegExpPrototypeExec(dotdotRe, str) !== null) + path = pathModule.normalize(str); + } validateRmOptions(path, options, false, (err, options) => { if (err) { @@ -1202,8 +1214,17 @@ function rm(path, options, callback) { * @returns {void} */ function rmSync(path, options) { + path = getValidatedPath(path); + if (typeof path === 'string') { + if (SideEffectFreeRegExpPrototypeExec(dotdotRe, path) !== null) + path = pathModule.normalize(path); + } else if (isArrayBufferView(path)) { + const str = BufferToString(path, 'utf8'); + if (SideEffectFreeRegExpPrototypeExec(dotdotRe, str) !== null) + path = pathModule.normalize(str); + } const opts = validateRmOptionsSync(path, options, false); - return binding.rmSync(getValidatedPath(path), opts.maxRetries, opts.recursive, opts.retryDelay); + return binding.rmSync(path, opts.maxRetries, opts.recursive, opts.retryDelay); } /** diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 2f95c4b79e17fd..6aea189d80acd3 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -17,6 +17,7 @@ const { Symbol, SymbolAsyncDispose, Uint8Array, + uncurryThis, } = primordials; const { fs: constants } = internalBinding('constants'); @@ -30,6 +31,7 @@ const { const binding = internalBinding('fs'); const { Buffer } = require('buffer'); +const BufferToString = uncurryThis(Buffer.prototype.toString); const { AbortError, @@ -92,6 +94,7 @@ const { promisify, isWindows, isMacOS, + SideEffectFreeRegExpPrototypeExec, } = require('internal/util'); const EventEmitter = require('events'); const { StringDecoder } = require('string_decoder'); @@ -102,6 +105,10 @@ const assert = require('internal/assert'); const permission = require('internal/process/permission'); +// Matches a dotdot path component (e.g. `..`, `/../`, `../`) but not +// filenames with consecutive dots like `a..b`. +const dotdotRe = /(^|[\\/])\.\.(?:[\\/]|$)/; + const kHandle = Symbol('kHandle'); const kFd = Symbol('kFd'); const kRefs = Symbol('kRefs'); @@ -802,6 +809,14 @@ async function ftruncate(handle, len = 0) { async function rm(path, options) { path = getValidatedPath(path); + if (typeof path === 'string') { + if (SideEffectFreeRegExpPrototypeExec(dotdotRe, path) !== null) + path = pathModule.normalize(path); + } else if (isArrayBufferView(path)) { + const str = BufferToString(path, 'utf8'); + if (SideEffectFreeRegExpPrototypeExec(dotdotRe, str) !== null) + path = pathModule.normalize(str); + } options = await validateRmOptionsPromise(path, options, false); return lazyRimRaf()(path, options); } diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 4104c2e948da0e..c5de1ff66bc577 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -569,3 +569,38 @@ if (isGitPresent) { } } } + +// Test that rm/rmSync normalize '.' and '..' in paths before processing. +// Regression test for https://github.com/nodejs/node/issues/61958 +// +// Each variant gets its own base directory to avoid async/sync races. +// The weird path is constructed by joining components with path.sep so that +// the '..' and '.' are preserved and not pre-normalized by path.join. +{ + // --- rmSync: /a/b/../. should remove /a entirely --- + const base = nextDirPath('dotdot-sync'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep); + fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true })); + assert.strictEqual(fs.existsSync(path.join(base, 'a')), false); +} + +{ + // --- fs.rm (callback): same path construction --- + const base = nextDirPath('dotdot-cb'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep); + fs.rm(weirdPath, common.mustNotMutateObjectDeep({ recursive: true }), common.mustSucceed(() => { + assert.strictEqual(fs.existsSync(path.join(base, 'a')), false); + })); +} + +{ + // --- fs.promises.rm: same path construction --- + const base = nextDirPath('dotdot-prom'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep); + fs.promises.rm(weirdPath, common.mustNotMutateObjectDeep({ recursive: true })).then(common.mustCall(() => { + assert.strictEqual(fs.existsSync(path.join(base, 'a')), false); + })); +}