Skip to content

Commit fe56eec

Browse files
authored
Merge pull request #199 from nodesource/minwoo/poc
[1.4.1] Avoiding npm substitution attacks & upgrade more deps
2 parents 414ea1a + 20db886 commit fe56eec

9 files changed

Lines changed: 1072 additions & 787 deletions

File tree

commands/install.js

Lines changed: 120 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
// In order to work around windows shell issues...
44
const spawn = require('cross-spawn')
5-
const { getValue } = require('../lib/config')
5+
const { getValue, setValue } = require('../lib/config')
6+
const clientRequest = require('../lib/client-request')
67
const { queryReadline, reversedSplit } = require('../lib/util')
78
const details = require('./details')
89
const {
@@ -11,20 +12,123 @@ const {
1112
failure
1213
} = require('../lib/ncm-style')
1314
const { helpHeader } = require('../lib/help')
15+
const path = require('path')
16+
const rc = require('rc')
1417
const chalk = require('chalk')
18+
1519
const L = console.log
1620
const E = console.error
1721

1822
module.exports = install
1923
module.exports.optionsList = optionsList
2024

25+
const publicRegistryUrl = 'https://registry.npmjs.org/'
26+
const cachedPublicPackages = getValue('cachedPublicPackages')
27+
28+
const registryUrl = scope => {
29+
const result = rc('npm', { registry: publicRegistryUrl })
30+
const url = result[`${scope}:registry`] || result.config_registry || result.registry
31+
return url.slice(-1) === '/' ? url : `${url}/`
32+
}
33+
34+
const scannerLog = () =>
35+
L(chalk`[NCM::SECURITY] Scanning npm dependency substitution vulnerabilities...`)
36+
37+
const concurrentTaskQueue = []
38+
const verificationTask = async ({ name, privateRegistryUrl }) => {
39+
try {
40+
L(`[NCM::SECURITY] Verifying the package "${name}"`)
41+
42+
// skip cached or scoped packages
43+
if (cachedPublicPackages.includes(name) || name.includes('/')) return
44+
45+
const { body: { versions = {} } } = await clientRequest({
46+
method: 'GET',
47+
uri: `${privateRegistryUrl}${name}`,
48+
json: true,
49+
opted: true
50+
})
51+
let verLen = Object.keys(versions).length
52+
if (!verLen) return
53+
54+
const { integrity: privateIntegrity } = versions[(Object.keys(versions)[verLen - 1])].dist
55+
if (!privateIntegrity) return
56+
57+
const { body: { versions: pubVers = {} } } = await clientRequest({
58+
method: 'GET',
59+
uri: `${publicRegistryUrl}${name}`,
60+
json: true,
61+
opted: true
62+
})
63+
verLen = Object.keys(pubVers).length
64+
if (!verLen) return
65+
66+
const { integrity: publicIntegrity } = pubVers[(Object.keys(pubVers)[verLen - 1])].dist
67+
68+
if (!publicIntegrity) return
69+
if (privateIntegrity !== publicIntegrity) {
70+
E(chalk.red(`[NCM::SECURITY ISSUE DETECTED] "${name}" is vulnerable to npm substitution attacks. Use scopes for the internal packages to fix this`))
71+
process.exit(1)
72+
} else {
73+
cachedPublicPackages.push(name)
74+
setValue('cachedPublicPackages', [...new Set(cachedPublicPackages)])
75+
}
76+
} catch (err) {
77+
E(err)
78+
}
79+
}
80+
2181
async function install (argv, arg1, arg2, arg3) {
2282
const childArgv = Array.from(process.argv.slice(3))
83+
const regUrl = registryUrl()
84+
const privateRegistryUrl = regUrl.endsWith('/') ? regUrl : `${regUrl}/`
2385
let name
2486
let version
87+
2588
if (!arg1) {
26-
printHelp()
27-
process.exitCode = 1
89+
try {
90+
scannerLog()
91+
92+
let deps
93+
try {
94+
deps = require(path.join(process.cwd(), 'package.json'))
95+
} catch (_) {} // skipped by below if there's no package.json
96+
97+
if (deps) {
98+
const pkgTree = [...new Set(Object.keys({
99+
...(deps.dependencies || {}),
100+
...(deps.devDependencies || {})
101+
}))]
102+
103+
pkgTree.forEach(async name =>
104+
concurrentTaskQueue.push(verificationTask({ name, privateRegistryUrl }))
105+
)
106+
107+
try {
108+
await Promise.all(concurrentTaskQueue)
109+
} catch (pErr) {
110+
E(pErr)
111+
}
112+
}
113+
} catch (err) {
114+
E(err)
115+
}
116+
117+
L(chalk`[NCM::SECURITY] Passed. Now installing...`)
118+
L()
119+
120+
const args = [getValue('installCmd'), '', ...childArgv]
121+
const bin = getValue('installBin')
122+
const cp = spawn(bin, args, { stdio: 'inherit' })
123+
124+
cp.on('error', error => {
125+
E()
126+
E(chalk`{COLORS.red ${error}}`)
127+
E()
128+
})
129+
cp.on('exit', code => {
130+
process.exitCode = code
131+
})
28132
return
29133
} else if (arg1.lastIndexOf('@') > 0 && !arg2 && !arg3) {
30134
childArgv.splice(childArgv.indexOf(arg1), 1)
@@ -46,6 +150,19 @@ async function install (argv, arg1, arg2, arg3) {
46150
return
47151
}
48152

153+
try {
154+
scannerLog()
155+
156+
concurrentTaskQueue.push(verificationTask({ name, privateRegistryUrl }))
157+
try {
158+
await Promise.all(concurrentTaskQueue)
159+
} catch (pErr) {
160+
E(pErr)
161+
}
162+
} catch (err) {
163+
E(err)
164+
}
165+
49166
await details(argv, arg1, arg2, arg3)
50167

51168
const good = process.exitCode === 0 || process.exitCode === undefined

commands/report.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,23 @@ async function report (argv, _dir) {
115115
return
116116
}
117117

118+
const {
119+
name: pkgName,
120+
version: pkgVersion
121+
} = require(path.join(__dirname, '..', 'package.json'))
122+
123+
let nestedPkgName, nestedPkgVersion
124+
try {
125+
const {
126+
name: _nestedPkgName,
127+
version: _nestedPkgVersion
128+
} = require(`${dir}/package.json`)
129+
nestedPkgName = _nestedPkgName
130+
nestedPkgVersion = _nestedPkgVersion
131+
} catch (_) {}
132+
133+
const isNested = pkgName === nestedPkgName && pkgVersion === nestedPkgVersion
134+
118135
for (let { name, version, scores, published } of data) {
119136
let maxSeverity = 0
120137
let license
@@ -147,6 +164,10 @@ async function report (argv, _dir) {
147164
version = '0.0.0-UNKNOWN-VERSION'
148165
}
149166

167+
if (isNested && !!maxSeverity) {
168+
continue
169+
}
170+
150171
pkgScores.push({
151172
name,
152173
version,

lib/client-request.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ async function ClientRequestExtended (options) {
5050
}
5151

5252
// Any non- 200-series code
53-
if (response.statusCode < 200 || response.statusCode >= 300) {
53+
if (!options.opted && (response.statusCode < 200 || response.statusCode >= 300)) {
5454
response.destroy()
5555

5656
let error

lib/config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ const keys = {
1414
org: 'default',
1515
orgId: '1',
1616
policy: ' ',
17-
policyId: ' '
17+
policyId: ' ',
18+
cachedPublicPackages: []
1819
}
1920

2021
module.exports = {

lib/report/short.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ function shortReport (report, whitelist, dir, argv) {
5555
L()
5656
}
5757

58+
let maxSeverityOfReport = 0
59+
report.forEach(({ maxSeverity }) => {
60+
maxSeverityOfReport = Math.max(maxSeverity, maxSeverityOfReport)
61+
})
62+
5863
if (filterCompliance || filterSecurity || filterLevel > 0) {
5964
const filterFormat = formatFilterOptions(filterOptions)
6065
if (whitelist.length > 0) {
@@ -76,7 +81,7 @@ function shortReport (report, whitelist, dir, argv) {
7681
filterOptions
7782
)
7883
}
79-
} else {
84+
} else if (maxSeverityOfReport > 0) {
8085
moduleList(report.slice(0, 5), 'Top 5: Highest Risk Modules')
8186
}
8287
}

0 commit comments

Comments
 (0)