diff --git a/__tests__/__snapshots__/normalize-manifest.js.snap b/__tests__/__snapshots__/normalize-manifest.js.snap index c5a78707da..eb903b98d4 100644 --- a/__tests__/__snapshots__/normalize-manifest.js.snap +++ b/__tests__/__snapshots__/normalize-manifest.js.snap @@ -60,6 +60,24 @@ Array [ ] `; +exports[`dangerous bin name: dangerous bin name 1`] = ` +Array [ + "foo: Invalid bin entry for \\"/tmp/foo\\" (in \\"foo\\").", + "foo: Invalid bin entry for \\"../tmp/foo\\" (in \\"foo\\").", + "foo: Invalid bin entry for \\"tmp/../../foo\\" (in \\"foo\\").", + "foo: No license field", +] +`; + +exports[`dangerous bin values: dangerous bin values 1`] = ` +Array [ + "foo: Invalid bin entry for \\"foo\\" (in \\"foo\\").", + "foo: Invalid bin entry for \\"bar\\" (in \\"foo\\").", + "foo: Invalid bin entry for \\"baz\\" (in \\"foo\\").", + "foo: No license field", +] +`; + exports[`dedupe all trivial dependencies: dedupe all trivial dependencies 1`] = ` Array [ "No license field", @@ -92,6 +110,7 @@ Array [ exports[`empty bin string: empty bin string 1`] = ` Array [ + "foo: Invalid bin field for \\"foo\\".", "foo: No license field", ] `; diff --git a/__tests__/fixtures/normalize-manifest/dangerous bin name/actual.json b/__tests__/fixtures/normalize-manifest/dangerous bin name/actual.json new file mode 100644 index 0000000000..d70ea69e80 --- /dev/null +++ b/__tests__/fixtures/normalize-manifest/dangerous bin name/actual.json @@ -0,0 +1,9 @@ +{ + "name": "foo", + "version": "", + "bin": { + "/tmp/foo": "main.js", + "../tmp/foo": "main.js", + "tmp/../../foo": "main.js" + } +} diff --git a/__tests__/fixtures/normalize-manifest/dangerous bin name/expected.json b/__tests__/fixtures/normalize-manifest/dangerous bin name/expected.json new file mode 100644 index 0000000000..80ce110c76 --- /dev/null +++ b/__tests__/fixtures/normalize-manifest/dangerous bin name/expected.json @@ -0,0 +1,5 @@ +{ + "name": "foo", + "version": "", + "bin": {} +} diff --git a/__tests__/fixtures/normalize-manifest/dangerous bin values/actual.json b/__tests__/fixtures/normalize-manifest/dangerous bin values/actual.json new file mode 100644 index 0000000000..a8e335abb3 --- /dev/null +++ b/__tests__/fixtures/normalize-manifest/dangerous bin values/actual.json @@ -0,0 +1,9 @@ +{ + "name": "foo", + "version": "", + "bin": { + "foo": "../../../../../foo", + "bar": "/hello/world", + "baz": "./foo/bar/../../../../../../foo" + } +} diff --git a/__tests__/fixtures/normalize-manifest/dangerous bin values/expected.json b/__tests__/fixtures/normalize-manifest/dangerous bin values/expected.json new file mode 100644 index 0000000000..80ce110c76 --- /dev/null +++ b/__tests__/fixtures/normalize-manifest/dangerous bin values/expected.json @@ -0,0 +1,5 @@ +{ + "name": "foo", + "version": "", + "bin": {} +} diff --git a/__tests__/fixtures/normalize-manifest/empty bin string/expected.json b/__tests__/fixtures/normalize-manifest/empty bin string/expected.json index 7c894d5fd8..15a2224ef8 100644 --- a/__tests__/fixtures/normalize-manifest/empty bin string/expected.json +++ b/__tests__/fixtures/normalize-manifest/empty bin string/expected.json @@ -1,5 +1,4 @@ { "name": "foo", - "version": "", - "bin": "" + "version": "" } diff --git a/src/reporters/lang/en.js b/src/reporters/lang/en.js index 100c1692e9..5175666d90 100644 --- a/src/reporters/lang/en.js +++ b/src/reporters/lang/en.js @@ -26,7 +26,8 @@ const messages = { couldntClearPackageFromCache: "Couldn't clear package $0 from cache", clearedPackageFromCache: 'Cleared package $0 from cache', packWroteTarball: 'Wrote tarball to $0.', - + invalidBinField: 'Invalid bin field for $0.', + invalidBinEntry: 'Invalid bin entry for $1 (in $0).', helpExamples: ' Examples:\n$0\n', helpCommands: ' Commands:\n$0\n', helpCommandsMore: ' Run `$0` for more information on specific commands.', diff --git a/src/util/normalize-manifest/fix.js b/src/util/normalize-manifest/fix.js index 95d19262a1..7d9796dee3 100644 --- a/src/util/normalize-manifest/fix.js +++ b/src/util/normalize-manifest/fix.js @@ -2,7 +2,7 @@ import {MANIFEST_FIELDS} from '../../constants'; import type {Reporter} from '../../reporters/index.js'; -import {isValidLicense} from './util.js'; +import {isValidBin, isValidLicense} from './util.js'; import {normalizePerson, extractDescription} from './util.js'; import {hostedGitFragmentToGitUrl} from '../../resolvers/index.js'; import inferLicense from './infer-license.js'; @@ -12,6 +12,8 @@ const semver = require('semver'); const path = require('path'); const url = require('url'); +const VALID_BIN_KEYS = /^[a-z0-9_-]+$/i; + const LICENSE_RENAMES: {[key: string]: ?string} = { 'MIT/X11': 'MIT', X11: 'MIT', @@ -159,6 +161,24 @@ export default (async function( info.bin = {[name]: info.bin}; } + // Validate that the bin entries reference only files within their package, and that + // their name is a valid file name + if (typeof info.bin === 'object' && info.bin !== null) { + const bin: Object = info.bin; + for (const key of Object.keys(bin)) { + const target = bin[key]; + if (!VALID_BIN_KEYS.test(key) || !isValidBin(target)) { + delete bin[key]; + warn(reporter.lang('invalidBinEntry', info.name, key)); + } else { + bin[key] = path.normalize(target); + } + } + } else if (typeof info.bin !== 'undefined') { + delete info.bin; + warn(reporter.lang('invalidBinField', info.name)); + } + // bundleDependencies is an alias for bundledDependencies if (info.bundledDependencies) { info.bundleDependencies = info.bundledDependencies; diff --git a/src/util/normalize-manifest/util.js b/src/util/normalize-manifest/util.js index 73f904882c..7d30f6106b 100644 --- a/src/util/normalize-manifest/util.js +++ b/src/util/normalize-manifest/util.js @@ -2,12 +2,19 @@ import type {PersonObject} from '../../types.js'; +const path = require('path'); const validateLicense = require('validate-npm-package-license'); +const PARENT_PATH = /^\.\.([\\\/]|$)/; + export function isValidLicense(license: string): boolean { return !!license && validateLicense(license).validForNewPackages; } +export function isValidBin(bin: string): boolean { + return !path.isAbsolute(bin) && !PARENT_PATH.test(path.normalize(bin)); +} + export function stringifyPerson(person: mixed): any { if (!person || typeof person !== 'object') { return person;