diff --git a/.changeset/kind-boxes-shake.md b/.changeset/kind-boxes-shake.md new file mode 100644 index 0000000000..0a0e7e0bc3 --- /dev/null +++ b/.changeset/kind-boxes-shake.md @@ -0,0 +1,6 @@ +--- +"@pnpm/package-bins": patch +"pnpm": patch +--- + +Fixed a path traversal vulnerability in pnpm's bin linking. Bin names starting with `@` bypassed validation, and after scope normalization, path traversal sequences like `../../` remained intact. \ No newline at end of file diff --git a/pkg-manager/package-bins/src/index.ts b/pkg-manager/package-bins/src/index.ts index 08cd4a0297..7e988d2b93 100644 --- a/pkg-manager/package-bins/src/index.ts +++ b/pkg-manager/package-bins/src/index.ts @@ -39,24 +39,21 @@ async function findFiles (dir: string): Promise { } } -function commandsFromBin (bin: PackageBin, pkgName: string, pkgPath: string) { - if (typeof bin === 'string') { - return [ - { - name: normalizeBinName(pkgName), - path: path.join(pkgPath, bin), - }, - ] +function commandsFromBin (bin: PackageBin, pkgName: string, pkgPath: string): Command[] { + const cmds: Command[] = [] + for (const [commandName, binRelativePath] of typeof bin === 'string' ? [[pkgName, bin]] : Object.entries(bin)) { + const binName = commandName[0] === '@' + ? commandName.slice(commandName.indexOf('/') + 1) + : commandName + // Validate: must be safe (no path traversal) - only allow URL-safe chars or $ + if (binName !== encodeURIComponent(binName) && binName !== '$') { + continue + } + const binPath = path.join(pkgPath, binRelativePath) + if (!isSubdir(pkgPath, binPath)) { + continue + } + cmds.push({ name: binName, path: binPath }) } - return Object.keys(bin) - .filter((commandName) => encodeURIComponent(commandName) === commandName || commandName === '$' || commandName[0] === '@') - .map((commandName) => ({ - name: normalizeBinName(commandName), - path: path.join(pkgPath, bin[commandName]), - })) - .filter((cmd) => isSubdir(pkgPath, cmd.path)) -} - -function normalizeBinName (name: string): string { - return name[0] === '@' ? name.slice(name.indexOf('/') + 1) : name + return cmds } diff --git a/pkg-manager/package-bins/test/index.ts b/pkg-manager/package-bins/test/index.ts index 40ecf44a81..95b90d86c0 100644 --- a/pkg-manager/package-bins/test/index.ts +++ b/pkg-manager/package-bins/test/index.ts @@ -126,3 +126,21 @@ test('get bin from scoped bin name', async () => { ] ) }) + +test('skip scoped bin names with path traversal', async () => { + expect( + await getBinsFromPackageManifest({ + name: 'malicious', + version: '1.0.0', + bin: { + '@scope/../../.npmrc': './malicious.js', + '@scope/../etc/passwd': './evil.js', + '@scope/legit': './good.js', + }, + }, process.cwd())).toStrictEqual([ + { + name: 'legit', + path: path.resolve('good.js'), + }, + ]) +})