Skip to content

Commit 726cfd6

Browse files
authored
Untrusted deserialization vulnerability detection (#5062)
* Untrusted deserialization vulnerability detection * use node-serialize most used version * fix sorting sensitive analyzers * remove DB source
1 parent a77283c commit 726cfd6

10 files changed

Lines changed: 116 additions & 16 deletions

File tree

.github/workflows/appsec.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,3 +298,17 @@ jobs:
298298
- uses: ./.github/actions/node/latest
299299
- run: yarn test:appsec:plugins:ci
300300
- uses: codecov/codecov-action@v3
301+
302+
node-serialize:
303+
runs-on: ubuntu-latest
304+
env:
305+
PLUGINS: node-serialize
306+
steps:
307+
- uses: actions/checkout@v4
308+
- uses: ./.github/actions/node/setup
309+
- uses: ./.github/actions/install
310+
- uses: ./.github/actions/node/oldest
311+
- run: yarn test:appsec:plugins:ci
312+
- uses: ./.github/actions/node/latest
313+
- run: yarn test:appsec:plugins:ci
314+
- uses: codecov/codecov-action@v3

packages/datadog-instrumentations/src/helpers/hooks.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ module.exports = {
8888
mysql2: () => require('../mysql2'),
8989
net: () => require('../net'),
9090
next: () => require('../next'),
91+
'node-serialize': () => require('../node-serialize'),
9192
'node:child_process': () => require('../child_process'),
9293
'node:crypto': () => require('../crypto'),
9394
'node:dns': () => require('../dns'),
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict'
2+
3+
const shimmer = require('../../datadog-shimmer')
4+
const { channel, addHook } = require('./helpers/instrument')
5+
6+
const nodeUnserializeCh = channel('datadog:node-serialize:unserialize:start')
7+
8+
function wrapUnserialize (serialize) {
9+
return function wrappedUnserialize (obj) {
10+
if (nodeUnserializeCh.hasSubscribers) {
11+
nodeUnserializeCh.publish({ obj })
12+
}
13+
14+
return serialize.apply(this, arguments)
15+
}
16+
}
17+
18+
addHook({ name: 'node-serialize', versions: ['0.0.4'] }, serialize => {
19+
shimmer.wrap(serialize, 'unserialize', wrapUnserialize)
20+
21+
return serialize
22+
})

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ module.exports = {
1717
SSRF: require('./ssrf-analyzer'),
1818
TEMPLATE_INJECTION_ANALYZER: require('./template-injection-analyzer'),
1919
UNVALIDATED_REDIRECT_ANALYZER: require('./unvalidated-redirect-analyzer'),
20+
UNTRUSTED_DESERIALIZATION_ANALYZER: require('./untrusted-deserialization-analyzer'),
2021
WEAK_CIPHER_ANALYZER: require('./weak-cipher-analyzer'),
2122
WEAK_HASH_ANALYZER: require('./weak-hash-analyzer'),
2223
WEAK_RANDOMNESS_ANALYZER: require('./weak-randomness-analyzer'),
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict'
2+
3+
const InjectionAnalyzer = require('./injection-analyzer')
4+
const { UNTRUSTED_DESERIALIZATION } = require('../vulnerabilities')
5+
6+
class UntrustedDeserializationAnalyzer extends InjectionAnalyzer {
7+
constructor () {
8+
super(UNTRUSTED_DESERIALIZATION)
9+
}
10+
11+
onConfigure () {
12+
this.addSub('datadog:node-serialize:unserialize:start', ({ obj }) => this.analyze(obj))
13+
}
14+
}
15+
16+
module.exports = new UntrustedDeserializationAnalyzer()

packages/dd-trace/src/appsec/iast/vulnerabilities-formatter/evidence-redaction/sensitive-handler.js

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,20 @@ class SensitiveHandler {
2525

2626
this._sensitiveAnalyzers = new Map()
2727
this._sensitiveAnalyzers.set(vulnerabilities.CODE_INJECTION, taintedRangeBasedSensitiveAnalyzer)
28-
this._sensitiveAnalyzers.set(vulnerabilities.TEMPLATE_INJECTION, taintedRangeBasedSensitiveAnalyzer)
2928
this._sensitiveAnalyzers.set(vulnerabilities.COMMAND_INJECTION, commandSensitiveAnalyzer)
30-
this._sensitiveAnalyzers.set(vulnerabilities.NOSQL_MONGODB_INJECTION, jsonSensitiveAnalyzer)
29+
this._sensitiveAnalyzers.set(vulnerabilities.HARDCODED_PASSWORD, (evidence) => {
30+
return hardcodedPasswordAnalyzer(evidence, this._valuePattern)
31+
})
32+
this._sensitiveAnalyzers.set(vulnerabilities.HEADER_INJECTION, (evidence) => {
33+
return headerSensitiveAnalyzer(evidence, this._namePattern, this._valuePattern)
34+
})
3135
this._sensitiveAnalyzers.set(vulnerabilities.LDAP_INJECTION, ldapSensitiveAnalyzer)
36+
this._sensitiveAnalyzers.set(vulnerabilities.NOSQL_MONGODB_INJECTION, jsonSensitiveAnalyzer)
3237
this._sensitiveAnalyzers.set(vulnerabilities.SQL_INJECTION, sqlSensitiveAnalyzer)
3338
this._sensitiveAnalyzers.set(vulnerabilities.SSRF, urlSensitiveAnalyzer)
39+
this._sensitiveAnalyzers.set(vulnerabilities.TEMPLATE_INJECTION, taintedRangeBasedSensitiveAnalyzer)
40+
this._sensitiveAnalyzers.set(vulnerabilities.UNTRUSTED_DESERIALIZATION, taintedRangeBasedSensitiveAnalyzer)
3441
this._sensitiveAnalyzers.set(vulnerabilities.UNVALIDATED_REDIRECT, urlSensitiveAnalyzer)
35-
this._sensitiveAnalyzers.set(vulnerabilities.HEADER_INJECTION, (evidence) => {
36-
return headerSensitiveAnalyzer(evidence, this._namePattern, this._valuePattern)
37-
})
38-
this._sensitiveAnalyzers.set(vulnerabilities.HARDCODED_PASSWORD, (evidence) => {
39-
return hardcodedPasswordAnalyzer(evidence, this._valuePattern)
40-
})
4142
}
4243

4344
isSensibleName (name) {

packages/dd-trace/src/appsec/iast/vulnerabilities.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ module.exports = {
1515
SSRF: 'SSRF',
1616
TEMPLATE_INJECTION: 'TEMPLATE_INJECTION',
1717
UNVALIDATED_REDIRECT: 'UNVALIDATED_REDIRECT',
18+
UNTRUSTED_DESERIALIZATION: 'UNTRUSTED_DESERIALIZATION',
1819
WEAK_CIPHER: 'WEAK_CIPHER',
1920
WEAK_HASH: 'WEAK_HASH',
2021
WEAK_RANDOMNESS: 'WEAK_RANDOMNESS',
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict'
2+
3+
const { prepareTestServerForIast } = require('../utils')
4+
const { storage } = require('../../../../../datadog-core')
5+
const iastContextFunctions = require('../../../../src/appsec/iast/iast-context')
6+
const { newTaintedString } = require('../../../../src/appsec/iast/taint-tracking/operations')
7+
8+
describe('untrusted-deserialization-analyzer with node-serialize', () => {
9+
withVersions('node-serialize', 'node-serialize', version => {
10+
let obj
11+
before(() => {
12+
obj = JSON.stringify({ name: 'example' })
13+
})
14+
15+
describe('unserialize', () => {
16+
prepareTestServerForIast('untrusted deserialization analyzer',
17+
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
18+
let lib
19+
beforeEach(() => {
20+
lib = require(`../../../../../../versions/node-serialize@${version}`).get()
21+
})
22+
23+
testThatRequestHasVulnerability(() => {
24+
const store = storage.getStore()
25+
const iastContext = iastContextFunctions.getIastContext(store)
26+
const str = newTaintedString(iastContext, obj, 'query', 'Request')
27+
lib.unserialize(str)
28+
}, 'UNTRUSTED_DESERIALIZATION')
29+
30+
testThatRequestHasNoVulnerability(() => {
31+
lib.unserialize(obj)
32+
}, 'UNTRUSTED_DESERIALIZATION')
33+
})
34+
})
35+
})
36+
})

packages/dd-trace/test/appsec/iast/vulnerability-formatter/index.spec.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ const excludedTests = [
1010
'Query with single quoted string literal and null source', // does not apply
1111
'Redacted source that needs to be truncated', // not implemented yet
1212
'CODE_INJECTION - Tainted range based redaction - with null source ', // does not apply
13-
'TEMPLATE_INJECTION - Tainted range based redaction - with null source ' // does not apply
13+
'TEMPLATE_INJECTION - Tainted range based redaction - with null source ', // does not apply
14+
'UNTRUSTED_DESERIALIZATION - Tainted range based redaction - with null source ' // does not apply
1415
]
1516

1617
function doTest (testCase, parameters) {

packages/dd-trace/test/appsec/iast/vulnerability-formatter/resources/evidence-redaction-suite.json

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2912,7 +2912,8 @@
29122912
"XSS",
29132913
"CODE_INJECTION",
29142914
"EMAIL_HTML_INJECTION",
2915-
"TEMPLATE_INJECTION"
2915+
"TEMPLATE_INJECTION",
2916+
"UNTRUSTED_DESERIALIZATION"
29162917
]
29172918
},
29182919
"input": [
@@ -2971,7 +2972,8 @@
29712972
"XSS",
29722973
"CODE_INJECTION",
29732974
"EMAIL_HTML_INJECTION",
2974-
"TEMPLATE_INJECTION"
2975+
"TEMPLATE_INJECTION",
2976+
"UNTRUSTED_DESERIALIZATION"
29752977
]
29762978
},
29772979
"input": [
@@ -3032,7 +3034,8 @@
30323034
"XSS",
30333035
"CODE_INJECTION",
30343036
"EMAIL_HTML_INJECTION",
3035-
"TEMPLATE_INJECTION"
3037+
"TEMPLATE_INJECTION",
3038+
"UNTRUSTED_DESERIALIZATION"
30363039
]
30373040
},
30383041
"input": [
@@ -3087,7 +3090,8 @@
30873090
"XSS",
30883091
"CODE_INJECTION",
30893092
"EMAIL_HTML_INJECTION",
3090-
"TEMPLATE_INJECTION"
3093+
"TEMPLATE_INJECTION",
3094+
"UNTRUSTED_DESERIALIZATION"
30913095
]
30923096
},
30933097
"input": [
@@ -3167,7 +3171,8 @@
31673171
"XSS",
31683172
"CODE_INJECTION",
31693173
"EMAIL_HTML_INJECTION",
3170-
"TEMPLATE_INJECTION"
3174+
"TEMPLATE_INJECTION",
3175+
"UNTRUSTED_DESERIALIZATION"
31713176
]
31723177
},
31733178
"input": [
@@ -3244,7 +3249,8 @@
32443249
"XSS",
32453250
"CODE_INJECTION",
32463251
"EMAIL_HTML_INJECTION",
3247-
"TEMPLATE_INJECTION"
3252+
"TEMPLATE_INJECTION",
3253+
"UNTRUSTED_DESERIALIZATION"
32483254
]
32493255
},
32503256
"input": [
@@ -3318,7 +3324,8 @@
33183324
"XSS",
33193325
"CODE_INJECTION",
33203326
"EMAIL_HTML_INJECTION",
3321-
"TEMPLATE_INJECTION"
3327+
"TEMPLATE_INJECTION",
3328+
"UNTRUSTED_DESERIALIZATION"
33223329
]
33233330
},
33243331
"input": [

0 commit comments

Comments
 (0)