diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index b099d4d72c6a4..e2b32c8b80b64 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -9,7 +9,7 @@ const promiseAllRejectLate = require('promise-all-reject-late') const runScript = require('@npmcli/run-script') const { callLimit: promiseCallLimit } = require('promise-call-limit') const { depth: dfwalk } = require('treeverse') -const { dirname, resolve, relative, join, sep } = require('node:path') +const { dirname, resolve, relative, join, isAbsolute, sep } = require('node:path') const { log, time } = require('proc-log') const { existsSync, realpathSync } = require('node:fs') const { lstat, mkdir, readdir, readlink, rm, symlink } = require('node:fs/promises') @@ -792,9 +792,18 @@ module.exports = cls => class Reifier extends cls { const { path: patchPath, integrity } = node.patched // validate the patch file here too, since reify can run on an ideal tree that skipped resolvePatchedDependencies + // the path comes from the lockfile in that case, so re-apply the same containment check readPatch does + const patchAbs = resolve(this.path, patchPath) + const patchRel = relative(this.path, patchAbs) + if (!patchRel || patchRel.startsWith('..') || isAbsolute(patchRel)) { + throw Object.assign( + new Error(`patch path escapes the project: ${patchPath}`), + { code: 'EPATCHUNSAFE', path: patchPath, node: node.name } + ) + } let contents try { - contents = await readFile(resolve(this.path, patchPath)) + contents = await readFile(patchAbs) } catch { throw Object.assign( new Error(`patch file not found: ${patchPath}`), diff --git a/workspaces/arborist/test/arborist/reify-patch.js b/workspaces/arborist/test/arborist/reify-patch.js index 53ac56feb7a9e..b6dd771018741 100644 --- a/workspaces/arborist/test/arborist/reify-patch.js +++ b/workspaces/arborist/test/arborist/reify-patch.js @@ -379,6 +379,50 @@ t.test('a patched optional dependency still fails loudly on patch problems', asy 'optional patch failure is not swallowed by optional handling') }) +t.test('reify refuses a lockfile patch path that escapes the project', async t => { + // node.patched.path is read verbatim from the lockfile. On a pre-built ideal + // tree (loadVirtual, not buildIdealTree) resolvePatchedDependencies is skipped, + // so a crafted lockfile path reaches #applyPatch unvalidated. reify must apply + // the same containment check readPatch does, or the read escapes the project. + const registry = createRegistry(t) + const src = t.testdir({ + 'package.json': JSON.stringify({ name: PKG_NAME, version: PKG_VERSION }), + 'index.js': ORIGINAL, + }) + const manifest = registry.manifest({ name: PKG_NAME, packuments: [{ version: PKG_VERSION }] }) + await registry.tarball({ manifest: manifest.versions[PKG_VERSION], tarball: src }) + const integrity = manifest.versions[PKG_VERSION].dist.integrity + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + dependencies: { [PKG_NAME]: `^${PKG_VERSION}` }, + }), + 'package-lock.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + lockfileVersion: 4, + requires: true, + packages: { + '': { name: 'root', version: '1.0.0', dependencies: { [PKG_NAME]: `^${PKG_VERSION}` } }, + [`node_modules/${PKG_NAME}`]: { + version: PKG_VERSION, + resolved: `https://registry.npmjs.org/${PKG_NAME}/-/${PKG_NAME}-${PKG_VERSION}.tgz`, + integrity, + patched: { path: '../../escape.patch', integrity: 'sha512-x' }, + }, + }, + }), + }) + + const arb = newArb({ path }) + // build the ideal tree straight from the lockfile so resolvePatchedDependencies is skipped + arb.idealTree = await arb.loadVirtual() + await t.rejects(arb.reify(), { code: 'EPATCHUNSAFE' }, + 'a lockfile patch path outside the project is refused before it is read') +}) + t.test('restores node.patched from an existing v4 lockfile', async t => { const patchRel = `patches/${PKG_NAME}@${PKG_VERSION}.patch` const path = makeProject(t, {