diff --git a/workspaces/arborist/lib/patch.js b/workspaces/arborist/lib/patch.js index 51d3c1878600f..322b3896c24ca 100644 --- a/workspaces/arborist/lib/patch.js +++ b/workspaces/arborist/lib/patch.js @@ -29,6 +29,22 @@ const containedTarget = (cwd, file) => { if (!rel || rel.startsWith('..') || isAbsolute(rel)) { throw patchError(`patch path escapes the package directory: ${file}`, 'EPATCHUNSAFE', file) } + // Lexical containment is not enough: cwd holds a package extracted from a + // registry tarball, so a symlink the publisher planted there can redirect + // the write or delete outside the package even though rel looks contained. + // Reject a symlink at the target itself, and require the deepest existing + // ancestor's real path to stay inside the real package directory. + if (fs.lstatSync(target, { throwIfNoEntry: false })?.isSymbolicLink()) { + throw patchError(`patch path escapes the package directory: ${file}`, 'EPATCHUNSAFE', file) + } + let ancestor = dirname(target) + while (!fs.existsSync(ancestor)) { + ancestor = dirname(ancestor) + } + const realRel = relative(fs.realpathSync(cwd), fs.realpathSync(ancestor)) + if (realRel.startsWith('..') || isAbsolute(realRel)) { + throw patchError(`patch path escapes the package directory: ${file}`, 'EPATCHUNSAFE', file) + } return target } diff --git a/workspaces/arborist/test/patch.js b/workspaces/arborist/test/patch.js index 652d7252f00a1..f2ddb11f6fcc5 100644 --- a/workspaces/arborist/test/patch.js +++ b/workspaces/arborist/test/patch.js @@ -1,5 +1,5 @@ const t = require('tap') -const { readFileSync, existsSync } = require('node:fs') +const { readFileSync, existsSync, symlinkSync } = require('node:fs') const { resolve } = require('node:path') const { createTwoFilesPatch } = require('diff') const { applyPatchToDir, patchIntegrity } = require('../lib/patch.js') @@ -79,6 +79,45 @@ t.test('refuses to delete outside the package directory', async t => { ) }) +t.test('refuses to write through a symlink that escapes the package', async t => { + const dir = t.testdir({ + pkg: { 'index.js': 'x\n' }, + outside: {}, + }) + symlinkSync(resolve(dir, 'outside'), resolve(dir, 'pkg', 'evil')) + await t.rejects( + applyPatchToDir({ patch: filePatch('evil/pwned.js', '', 'pwned\n'), cwd: resolve(dir, 'pkg') }), + { code: 'EPATCHUNSAFE' } + ) + t.notOk(existsSync(resolve(dir, 'outside', 'pwned.js')), 'nothing written outside') +}) + +t.test('refuses to delete through a symlink that escapes the package', async t => { + const dir = t.testdir({ + pkg: { 'index.js': 'x\n' }, + outside: { 'secret.js': 'secret\n' }, + }) + symlinkSync(resolve(dir, 'outside'), resolve(dir, 'pkg', 'evil')) + await t.rejects( + applyPatchToDir({ patch: filePatch('evil/secret.js', 'secret\n', ''), cwd: resolve(dir, 'pkg') }), + { code: 'EPATCHUNSAFE' } + ) + t.ok(existsSync(resolve(dir, 'outside', 'secret.js')), 'outside file untouched') +}) + +t.test('refuses a target that is itself a symlink out of the package', async t => { + const dir = t.testdir({ + pkg: { 'index.js': 'x\n' }, + outside: { 'secret.js': 'const v = 1\n' }, + }) + symlinkSync(resolve(dir, 'outside', 'secret.js'), resolve(dir, 'pkg', 'link.js')) + await t.rejects( + applyPatchToDir({ patch: filePatch('link.js', 'const v = 1\n', 'const v = 2\n'), cwd: resolve(dir, 'pkg') }), + { code: 'EPATCHUNSAFE' } + ) + t.equal(readFileSync(resolve(dir, 'outside', 'secret.js'), 'utf8'), 'const v = 1\n', 'outside file untouched') +}) + t.test('delete fails when the file drifted from the diff', async t => { const dir = t.testdir({ 'gone.js': 'different content\n' }) await t.rejects(