Skip to content

Commit fd1dd7e

Browse files
iunanuauurienIlyasShabi
authored
[asm] IAST security controls (#5117)
* Security controls parser and secure marks for vulnerabilities * Use new NOSQL_MONGODB_INJECTION_MARK in nosql-injection-mongodb-analyzer * Config * first hooks * wrap object properties and more tests * Use dd-trace:moduleLoad(Start|End) channels * iterate object strings and more tests * fix parameter index, include createNewTainted flag and do not use PluginManager in the tests * Fix parameter index and include a test with incorrect index * Avoid to hook multiple times the same module and config tests * sql_injection_mark test * vulnerable ranges tests * fix windows paths * Upgrade taint-tracking to 3.3.0 * Fix * secure mark * add createNewTainted flag to addSecureMark * Use existing _isRangeSecure * supressed vulnerabilities metric * increment supressed vulnerability metric * typo * handle esm default export and filenames starting with file:// * esm integration tests * clean up * secure-marks tests * fix secure-marks generator test * fix config test * empty * check for repeated marks * Update packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * Update packages/dd-trace/src/appsec/iast/security-controls/index.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * Update packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * some suggestions * move _isRangeSecure to InjectionAnalyzer * Add programatically config option * index.d.ts * StoredInjectionAnalyzer * Update packages/dd-trace/test/appsec/iast/analyzers/command-injection-analyzer.spec.js Co-authored-by: ishabi <ilyas.shabi@datadoghq.com> * store control keys to avoid recreating the array * check visited before iterating * test suggestions * Update packages/dd-trace/src/appsec/iast/security-controls/parser.js Co-authored-by: Ilyas Shabi <ilyas.shabi@datadoghq.com> * lint * ritm test * clean up * Reject security control with non numeric parameters * fix parameter 0 * Update integration-tests/appsec/iast.esm-security-controls.spec.js Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> * suggestions * use legacy store * fix test * fix test * fix test --------- Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> Co-authored-by: ishabi <ilyas.shabi@datadoghq.com>
1 parent 784b6f3 commit fd1dd7e

40 files changed

+1589
-76
lines changed

index.d.ts

+5
Original file line numberDiff line numberDiff line change
@@ -2226,6 +2226,11 @@ declare namespace tracer {
22262226
*/
22272227
redactionValuePattern?: string,
22282228

2229+
/**
2230+
* Allows to enable security controls.
2231+
*/
2232+
securityControlsConfiguration?: string,
2233+
22292234
/**
22302235
* Specifies the verbosity of the sent telemetry. Default 'INFORMATION'
22312236
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
'use strict'
2+
3+
import childProcess from 'node:child_process'
4+
import express from 'express'
5+
import { sanitize } from './sanitizer.mjs'
6+
import sanitizeDefault from './sanitizer-default.mjs'
7+
import { validate, validateNotConfigured } from './validator.mjs'
8+
9+
const app = express()
10+
const port = process.env.APP_PORT || 3000
11+
12+
app.get('/cmdi-s-secure', (req, res) => {
13+
const command = sanitize(req.query.command)
14+
try {
15+
childProcess.execSync(command)
16+
} catch (e) {
17+
// ignore
18+
}
19+
20+
res.end()
21+
})
22+
23+
app.get('/cmdi-s-secure-comparison', (req, res) => {
24+
const command = sanitize(req.query.command)
25+
try {
26+
childProcess.execSync(command)
27+
} catch (e) {
28+
// ignore
29+
}
30+
31+
try {
32+
childProcess.execSync(req.query.command)
33+
} catch (e) {
34+
// ignore
35+
}
36+
37+
res.end()
38+
})
39+
40+
app.get('/cmdi-s-secure-default', (req, res) => {
41+
const command = sanitizeDefault(req.query.command)
42+
try {
43+
childProcess.execSync(command)
44+
} catch (e) {
45+
// ignore
46+
}
47+
48+
res.end()
49+
})
50+
51+
app.get('/cmdi-iv-insecure', (req, res) => {
52+
if (validateNotConfigured(req.query.command)) {
53+
childProcess.execSync(req.query.command)
54+
}
55+
56+
res.end()
57+
})
58+
59+
app.get('/cmdi-iv-secure', (req, res) => {
60+
if (validate(req.query.command)) {
61+
childProcess.execSync(req.query.command)
62+
}
63+
64+
res.end()
65+
})
66+
67+
app.listen(port, () => {
68+
process.send({ port })
69+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict'
2+
3+
function sanitizeDefault (input) {
4+
return input
5+
}
6+
7+
export default sanitizeDefault
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict'
2+
3+
export function sanitize (input) {
4+
return input
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict'
2+
3+
export function validate (input) {
4+
return true
5+
}
6+
7+
export function validateNotConfigured (input) {
8+
return true
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
'use strict'
2+
3+
const { createSandbox, spawnProc, FakeAgent } = require('../helpers')
4+
const path = require('path')
5+
const getPort = require('get-port')
6+
const Axios = require('axios')
7+
const { assert } = require('chai')
8+
9+
describe('ESM Security controls', () => {
10+
let axios, sandbox, cwd, appPort, appFile, agent, proc
11+
12+
before(async function () {
13+
this.timeout(process.platform === 'win32' ? 90000 : 30000)
14+
sandbox = await createSandbox(['express'])
15+
appPort = await getPort()
16+
cwd = sandbox.folder
17+
appFile = path.join(cwd, 'appsec', 'esm-security-controls', 'index.mjs')
18+
19+
axios = Axios.create({
20+
baseURL: `http://localhost:${appPort}`
21+
})
22+
})
23+
24+
after(async function () {
25+
await sandbox.remove()
26+
})
27+
28+
const nodeOptions = '--import dd-trace/initialize.mjs'
29+
30+
beforeEach(async () => {
31+
agent = await new FakeAgent().start()
32+
33+
proc = await spawnProc(appFile, {
34+
cwd,
35+
env: {
36+
DD_TRACE_AGENT_PORT: agent.port,
37+
APP_PORT: appPort,
38+
DD_IAST_ENABLED: 'true',
39+
DD_IAST_REQUEST_SAMPLING: '100',
40+
// eslint-disable-next-line no-multi-str
41+
DD_IAST_SECURITY_CONTROLS_CONFIGURATION: '\
42+
SANITIZER:COMMAND_INJECTION:appsec/esm-security-controls/sanitizer.mjs:sanitize;\
43+
SANITIZER:COMMAND_INJECTION:appsec/esm-security-controls/sanitizer-default.mjs;\
44+
INPUT_VALIDATOR:*:appsec/esm-security-controls/validator.mjs:validate',
45+
NODE_OPTIONS: nodeOptions
46+
}
47+
})
48+
})
49+
50+
afterEach(async () => {
51+
proc.kill()
52+
await agent.stop()
53+
})
54+
55+
it('test endpoint with iv not configured does have COMMAND_INJECTION vulnerability', async function () {
56+
await axios.get('/cmdi-iv-insecure?command=ls -la')
57+
58+
await agent.assertMessageReceived(({ payload }) => {
59+
const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
60+
spans.forEach(span => {
61+
assert.property(span.meta, '_dd.iast.json')
62+
assert.include(span.meta['_dd.iast.json'], '"COMMAND_INJECTION"')
63+
})
64+
}, null, 1, true)
65+
})
66+
67+
it('test endpoint sanitizer does not have COMMAND_INJECTION vulnerability', async () => {
68+
await axios.get('/cmdi-s-secure?command=ls -la')
69+
70+
await agent.assertMessageReceived(({ payload }) => {
71+
const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
72+
spans.forEach(span => {
73+
assert.notProperty(span.meta, '_dd.iast.json')
74+
assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
75+
})
76+
}, null, 1, true)
77+
})
78+
79+
it('test endpoint with default sanitizer does not have COMMAND_INJECTION vulnerability', async () => {
80+
await axios.get('/cmdi-s-secure-default?command=ls -la')
81+
82+
await agent.assertMessageReceived(({ payload }) => {
83+
const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
84+
spans.forEach(span => {
85+
assert.notProperty(span.meta, '_dd.iast.json')
86+
assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
87+
})
88+
}, null, 1, true)
89+
})
90+
91+
it('test endpoint with default sanitizer does have COMMAND_INJECTION with original tainted', async () => {
92+
await axios.get('/cmdi-s-secure-comparison?command=ls -la')
93+
94+
await agent.assertMessageReceived(({ payload }) => {
95+
const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
96+
spans.forEach(span => {
97+
assert.property(span.meta, '_dd.iast.json')
98+
assert.include(span.meta['_dd.iast.json'], '"COMMAND_INJECTION"')
99+
})
100+
}, null, 1, true)
101+
})
102+
103+
it('test endpoint with default sanitizer does have COMMAND_INJECTION vulnerability', async () => {
104+
await axios.get('/cmdi-s-secure-default?command=ls -la')
105+
106+
await agent.assertMessageReceived(({ payload }) => {
107+
const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
108+
spans.forEach(span => {
109+
assert.notProperty(span.meta, '_dd.iast.json')
110+
assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
111+
})
112+
}, null, 1, true)
113+
})
114+
115+
it('test endpoint with iv does not have COMMAND_INJECTION vulnerability', async () => {
116+
await axios.get('/cmdi-iv-secure?command=ls -la')
117+
118+
await agent.assertMessageReceived(({ payload }) => {
119+
const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
120+
spans.forEach(span => {
121+
assert.notProperty(span.meta, '_dd.iast.json')
122+
assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
123+
})
124+
}, null, 1, true)
125+
})
126+
})

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
"@datadog/libdatadog": "^0.4.0",
8585
"@datadog/native-appsec": "8.4.0",
8686
"@datadog/native-iast-rewriter": "2.8.0",
87-
"@datadog/native-iast-taint-tracking": "3.2.0",
87+
"@datadog/native-iast-taint-tracking": "3.3.0",
8888
"@datadog/native-metrics": "^3.1.0",
8989
"@datadog/pprof": "5.5.1",
9090
"@datadog/sketches-js": "^2.1.0",

packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
'use strict'
22

3-
const InjectionAnalyzer = require('./injection-analyzer')
43
const { CODE_INJECTION } = require('../vulnerabilities')
4+
const StoredInjectionAnalyzer = require('./stored-injection-analyzer')
55
const { INSTRUMENTED_SINK } = require('../telemetry/iast-metric')
66
const { storage } = require('../../../../../datadog-core')
77
const { getIastContext } = require('../iast-context')
88

9-
class CodeInjectionAnalyzer extends InjectionAnalyzer {
9+
class CodeInjectionAnalyzer extends StoredInjectionAnalyzer {
1010
constructor () {
1111
super(CODE_INJECTION)
1212
this.evalInstrumentedInc = false
@@ -31,10 +31,6 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer {
3131
this.addSub('datadog:vm:run-script:start', ({ code }) => this.analyze(code))
3232
this.addSub('datadog:vm:source-text-module:start', ({ code }) => this.analyze(code))
3333
}
34-
35-
_areRangesVulnerable () {
36-
return true
37-
}
3834
}
3935

4036
module.exports = new CodeInjectionAnalyzer()

packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js

+15-1
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,13 @@ const { SQL_ROW_VALUE } = require('../taint-tracking/source-types')
55

66
class InjectionAnalyzer extends Analyzer {
77
_isVulnerable (value, iastContext) {
8-
const ranges = value && getRanges(iastContext, value)
8+
let ranges = value && getRanges(iastContext, value)
99
if (ranges?.length > 0) {
10+
ranges = this._filterSecureRanges(ranges)
11+
if (!ranges?.length) {
12+
this._incrementSuppressedMetric(iastContext)
13+
}
14+
1015
return this._areRangesVulnerable(ranges)
1116
}
1217

@@ -21,6 +26,15 @@ class InjectionAnalyzer extends Analyzer {
2126
_areRangesVulnerable (ranges) {
2227
return ranges?.some(range => range.iinfo.type !== SQL_ROW_VALUE)
2328
}
29+
30+
_filterSecureRanges (ranges) {
31+
return ranges?.filter(range => !this._isRangeSecure(range))
32+
}
33+
34+
_isRangeSecure (range) {
35+
const { secureMarks } = range
36+
return (secureMarks & this._secureMark) === this._secureMark
37+
}
2438
}
2539

2640
module.exports = InjectionAnalyzer

packages/dd-trace/src/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.js

+11-24
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,13 @@ const InjectionAnalyzer = require('./injection-analyzer')
44
const { NOSQL_MONGODB_INJECTION } = require('../vulnerabilities')
55
const { getRanges, addSecureMark } = require('../taint-tracking/operations')
66
const { getNodeModulesPaths } = require('../path-line')
7-
const { getNextSecureMark } = require('../taint-tracking/secure-marks-generator')
87
const { storage } = require('../../../../../datadog-core')
98
const { getIastContext } = require('../iast-context')
109
const { HTTP_REQUEST_PARAMETER, HTTP_REQUEST_BODY } = require('../taint-tracking/source-types')
1110

1211
const EXCLUDED_PATHS_FROM_STACK = getNodeModulesPaths('mongodb', 'mongoose', 'mquery')
13-
const MONGODB_NOSQL_SECURE_MARK = getNextSecureMark()
14-
15-
function iterateObjectStrings (target, fn, levelKeys = [], depth = 20, visited = new Set()) {
16-
if (target !== null && typeof target === 'object') {
17-
Object.keys(target).forEach((key) => {
18-
const nextLevelKeys = [...levelKeys, key]
19-
const val = target[key]
20-
21-
if (typeof val === 'string') {
22-
fn(val, nextLevelKeys, target, key)
23-
} else if (depth > 0 && !visited.has(val)) {
24-
iterateObjectStrings(val, fn, nextLevelKeys, depth - 1, visited)
25-
visited.add(val)
26-
}
27-
})
28-
}
29-
}
12+
const { NOSQL_MONGODB_INJECTION_MARK } = require('../taint-tracking/secure-marks')
13+
const { iterateObjectStrings } = require('../utils')
3014

3115
class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
3216
constructor () {
@@ -88,7 +72,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
8872
const currentLevelKey = levelKeys[i]
8973

9074
if (i === levelsLength - 1) {
91-
parentObj[currentLevelKey] = addSecureMark(iastContext, value, MONGODB_NOSQL_SECURE_MARK)
75+
parentObj[currentLevelKey] = addSecureMark(iastContext, value, NOSQL_MONGODB_INJECTION_MARK)
9276
} else {
9377
parentObj = parentObj[currentLevelKey]
9478
}
@@ -106,7 +90,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
10690
if (iastContext) { // do nothing if we are not in an iast request
10791
iterateObjectStrings(sanitizedObject, function (value, levelKeys, parent, lastKey) {
10892
try {
109-
parent[lastKey] = addSecureMark(iastContext, value, MONGODB_NOSQL_SECURE_MARK)
93+
parent[lastKey] = addSecureMark(iastContext, value, NOSQL_MONGODB_INJECTION_MARK)
11094
} catch {
11195
// if it is a readonly property, do nothing
11296
}
@@ -121,8 +105,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
121105

122106
_isVulnerableRange (range) {
123107
const rangeType = range?.iinfo?.type
124-
const isVulnerableType = rangeType === HTTP_REQUEST_PARAMETER || rangeType === HTTP_REQUEST_BODY
125-
return isVulnerableType && (range.secureMarks & MONGODB_NOSQL_SECURE_MARK) !== MONGODB_NOSQL_SECURE_MARK
108+
return rangeType === HTTP_REQUEST_PARAMETER || rangeType === HTTP_REQUEST_BODY
126109
}
127110

128111
_isVulnerable (value, iastContext) {
@@ -137,10 +120,15 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
137120
const allRanges = []
138121

139122
iterateObjectStrings(value.filter, (val, nextLevelKeys) => {
140-
const ranges = getRanges(iastContext, val)
123+
let ranges = getRanges(iastContext, val)
141124
if (ranges?.length) {
142125
const filteredRanges = []
143126

127+
ranges = this._filterSecureRanges(ranges)
128+
if (!ranges.length) {
129+
this._incrementSuppressedMetric(iastContext)
130+
}
131+
144132
for (const range of ranges) {
145133
if (this._isVulnerableRange(range)) {
146134
isVulnerable = true
@@ -175,4 +163,3 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
175163
}
176164

177165
module.exports = new NosqlInjectionMongodbAnalyzer()
178-
module.exports.MONGODB_NOSQL_SECURE_MARK = MONGODB_NOSQL_SECURE_MARK

0 commit comments

Comments
 (0)