From 960ecde66f8c9876b7b05317150c2298f2d48ac7 Mon Sep 17 00:00:00 2001 From: ubeddulla khan Date: Sat, 4 Jul 2026 15:48:44 +0530 Subject: [PATCH] fix(stage): strip path separators from staged tarball filename --- lib/commands/stage/download.js | 7 +++-- test/lib/commands/stage/download.js | 44 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/commands/stage/download.js b/lib/commands/stage/download.js index e5b7711aee54d..379659c6062de 100644 --- a/lib/commands/stage/download.js +++ b/lib/commands/stage/download.js @@ -33,8 +33,11 @@ class StageDownload extends BaseCommand { const pkgContents = await getContents(manifest, data) logTar(pkgContents, { unicode, json, key: pkgContents.name }) - const safeName = pkgContents.name.replace('@', '').replace('/', '-') - const filename = `${safeName}-${pkgContents.version}-${stageId}.tgz` + // name and version come from the untrusted staged tarball's package.json, + // so strip every path separator (as libnpmpack does) before they reach the + // filesystem, otherwise a crafted version like ../../x escapes the cwd. + const filename = `${pkgContents.name}-${pkgContents.version}-${stageId}.tgz` + .replace(/^@/, '').replace(/[/\\]/g, '-') const dest = resolve(process.cwd(), filename) await writeFile(dest, data) diff --git a/test/lib/commands/stage/download.js b/test/lib/commands/stage/download.js index 807c1282edc86..a5ebf4d35cbb0 100644 --- a/test/lib/commands/stage/download.js +++ b/test/lib/commands/stage/download.js @@ -1,10 +1,12 @@ const t = require('tap') const fs = require('node:fs') +const os = require('node:os') const path = require('node:path') const { load: loadMockNpm } = require('../../../fixtures/mock-npm.js') const MockRegistry = require('@npmcli/mock-registry') const mockGlobals = require('@npmcli/mock-globals') const libpack = require('libnpmpack') +const tar = require('tar') const token = 'test-auth-token' const authConfig = { '//registry.npmjs.org/:_authToken': token } @@ -137,3 +139,45 @@ t.test('throws when tarball has no package.json', async t => { message: /Could not read package.json from tarball/, }) }) + +t.test('keeps the written tarball inside cwd for a crafted version', async t => { + const { npm, prefix } = await loadMockNpm(t, { + config: authConfig, + prefixDir: { + 'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }), + }, + }) + + // Build a staged tarball whose package.json version tries to walk out of cwd. + // Use an os tempdir here (not t.testdir) so we don't clobber mock-npm's prefix. + const tarDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stage-download-')) + t.teardown(() => fs.rmSync(tarDir, { recursive: true, force: true })) + fs.mkdirSync(path.join(tarDir, 'package')) + fs.writeFileSync(path.join(tarDir, 'package', 'package.json'), + JSON.stringify({ name: 'evil', version: '../evil-marker' })) + const chunks = [] + await new Promise((res, rej) => { + tar.c({ cwd: tarDir, gzip: false }, ['package']) + .on('data', c => chunks.push(c)) + .on('end', res) + .on('error', rej) + }) + const tarballData = Buffer.concat(chunks) + + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: token, + }) + registry.nock.get(`/-/stage/${stageId}/tarball`) + .reply(200, tarballData, { 'content-type': 'application/octet-stream' }) + + mockGlobals(t, { 'process.cwd': () => prefix }) + + await npm.exec('stage', ['download', stageId]) + + // separators are stripped, so the file lands inside the prefix... + t.ok(fs.existsSync(path.join(prefix, `evil-..-evil-marker-${stageId}.tgz`))) + // ...and nothing is written to the parent directory. + t.notOk(fs.existsSync(path.join(prefix, '..', `evil-marker-${stageId}.tgz`))) +})