Skip to content

Commit

Permalink
fix: apply securePath to package bin (#105)
Browse files Browse the repository at this point in the history
## What / Why

* Aligns path normalization logic when processing `bin` and `man` refs.
* Fixes out of scope path traversals for `bin`

```js
function unixifyPath (ref) {
  return ref.replace(/\\|:/g, '/')
}

function securePath (ref) {
  const secured = path.join('.', path.join('/', unixifyPath(ref)))
  return secured.startsWith('.') ? '' : secured
}

function secureAndUnixifyPath (ref) {
  return unixifyPath(securePath(ref))
}
```

## References
continues
[#100](#100 (comment)),
#104
  • Loading branch information
antongolub authored May 28, 2024
1 parent 46c563b commit 54756d2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 10 deletions.
24 changes: 14 additions & 10 deletions lib/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,14 @@ function normalizePackageBin (pkg, changes) {
changes?.push(`removed invalid "bin[${binKey}]"`)
continue
}
const base = path.join('/', path.basename(binKey.replace(/\\|:/g, '/'))).slice(1)
const base = path.basename(secureAndUnixifyPath(binKey))
if (!base) {
delete pkg.bin[binKey]
changes?.push(`removed invalid "bin[${binKey}]"`)
continue
}

const binTarget = path.join('/', pkg.bin[binKey].replace(/\\/g, '/'))
.replace(/\\/g, '/').slice(1)
const binTarget = secureAndUnixifyPath(pkg.bin[binKey])

if (!binTarget) {
delete pkg.bin[binKey]
Expand Down Expand Up @@ -90,7 +89,7 @@ function normalizePackageMan (pkg, changes) {
if (typeof man !== 'string') {
changes?.push(`removed invalid "man [${man}]"`)
} else {
mans.push(unixifyPath(securePath(unixifyPath(man))))
mans.push(secureAndUnixifyPath(man))
}
}

Expand All @@ -109,10 +108,6 @@ function isCorrectlyEncodedName (spec) {
spec === encodeURIComponent(spec)
}

function unixifyPath (ref) {
return ref.replace(/\\|:/g, '/')
}

function isValidScopedPackageName (spec) {
if (spec.charAt(0) !== '@') {
return false
Expand All @@ -128,8 +123,17 @@ function isValidScopedPackageName (spec) {
rest[1] === encodeURIComponent(rest[1])
}

function unixifyPath (ref) {
return ref.replace(/\\|:/g, '/')
}

function securePath (ref) {
return path.join('.', path.join('/', ref))
const secured = path.join('.', path.join('/', unixifyPath(ref)))
return secured.startsWith('.') ? '' : secured
}

function secureAndUnixifyPath (ref) {
return unixifyPath(securePath(ref))
}

// We don't want the `changes` array in here by default because this is a hot
Expand Down Expand Up @@ -356,7 +360,7 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
// expand directories.man
if (steps.includes('mans')) {
if (data.directories?.man && !data.man) {
const manDir = unixifyPath(securePath(unixifyPath(data.directories.man)))
const manDir = secureAndUnixifyPath(data.directories.man)
const cwd = path.resolve(pkg.path, manDir)
const files = await lazyLoadGlob()('**/*.[0-9]', { cwd })
data.man = files.map(man =>
Expand Down
24 changes: 24 additions & 0 deletions test/prepare.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,30 @@ for (const [name, testPrepare] of Object.entries(testMethods)) {
t.strictSame(content.bin, { echo: 'bin/echo' })
})

t.test('bin trim prefix', async t => {
const { content } = await testPrepare(t, ({
'package.json': JSON.stringify({
name: 'bin-test',
bin: '../../../../../bin/echo',
}),
bin: { echo: '#!/bin/sh\n\necho "hello world"' },
}))
t.strictSame(content.bin, { 'bin-test': 'bin/echo' })
})

t.test('bin handles back slashes', async t => {
const { content } = await testPrepare(t, ({
'package.json': JSON.stringify({
name: 'bin-test',
bin: {
echo: '..\\..\\..\\bin\\echo',
},
}),
bin: { echo: '#!/bin/sh\n\necho "hello world"' },
}))
t.strictSame(content.bin, { echo: 'bin/echo' })
})

t.test('directories.bin with bin', async t => {
const { content } = await testPrepare(t, ({
'package.json': JSON.stringify({
Expand Down

0 comments on commit 54756d2

Please sign in to comment.