Skip to content

Commit e0ac795

Browse files
IlyasShabiIlyasShabi
authored
Extended iast location fields (#5171)
* Extend iast location fields * improve iast test to check if frame have location * check that location do not have column * Exlude test without location * use official msgpack * Fix linter * send class and function from original location * fix test
1 parent c64020a commit e0ac795

File tree

9 files changed

+106
-40
lines changed

9 files changed

+106
-40
lines changed

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

+8-2
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,17 @@ class Analyzer extends SinkIastPlugin {
7575
if (locationFromSourceMap?.path) {
7676
originalLocation.path = locationFromSourceMap.path
7777
}
78+
7879
if (locationFromSourceMap?.line) {
7980
originalLocation.line = locationFromSourceMap.line
8081
}
81-
if (locationFromSourceMap?.column) {
82-
originalLocation.column = locationFromSourceMap.column
82+
83+
if (location?.class_name) {
84+
originalLocation.class = location.class_name
85+
}
86+
87+
if (location?.function) {
88+
originalLocation.method = location.function
8389
}
8490

8591
return originalLocation

‎packages/dd-trace/src/appsec/iast/vulnerabilities-formatter/index.js

+8-13
Original file line numberDiff line numberDiff line change
@@ -81,21 +81,16 @@ class VulnerabilityFormatter {
8181
}
8282

8383
formatVulnerability (vulnerability, sourcesIndexes, sources) {
84+
const { type, hash, stackId, evidence, location } = vulnerability
85+
8486
const formattedVulnerability = {
85-
type: vulnerability.type,
86-
hash: vulnerability.hash,
87-
stackId: vulnerability.stackId,
88-
evidence: this.formatEvidence(vulnerability.type, vulnerability.evidence, sourcesIndexes, sources),
89-
location: {
90-
spanId: vulnerability.location.spanId
91-
}
92-
}
93-
if (vulnerability.location.path) {
94-
formattedVulnerability.location.path = vulnerability.location.path
95-
}
96-
if (vulnerability.location.line) {
97-
formattedVulnerability.location.line = vulnerability.location.line
87+
type,
88+
hash,
89+
stackId,
90+
evidence: this.formatEvidence(type, evidence, sourcesIndexes, sources),
91+
location
9892
}
93+
9994
return formattedVulnerability
10095
}
10196

‎packages/dd-trace/src/appsec/iast/vulnerability-reporter.js

+1
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ function replaceCallSiteFromSourceMap (callsite) {
131131
if (line) {
132132
callsite.line = line
133133
}
134+
// We send the column in the stack trace but not in the vulnerability location
134135
if (column) {
135136
callsite.column = column
136137
}

‎packages/dd-trace/test/appsec/iast/analyzers/hsts-header-missing-analyzer.spec.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,23 @@ describe('hsts header missing analyzer', () => {
2727
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
2828
expect(vulnerabilities[0].evidence).to.be.undefined
2929
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
30-
}, makeRequestWithXFordwardedProtoHeader)
30+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
3131

3232
testThatRequestHasVulnerability((req, res) => {
3333
res.setHeader('content-type', 'text/html;charset=utf-8')
3434
res.end('<html><body><h1>Test</h1></body></html>')
3535
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
3636
expect(vulnerabilities[0].evidence).to.be.undefined
3737
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
38-
}, makeRequestWithXFordwardedProtoHeader)
38+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
3939

4040
testThatRequestHasVulnerability((req, res) => {
4141
res.setHeader('content-type', 'application/xhtml+xml')
4242
res.end('<html><body><h1>Test</h1></body></html>')
4343
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
4444
expect(vulnerabilities[0].evidence).to.be.undefined
4545
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
46-
}, makeRequestWithXFordwardedProtoHeader)
46+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
4747

4848
testThatRequestHasVulnerability((req, res) => {
4949
res.setHeader('content-type', 'text/html')
@@ -52,7 +52,7 @@ describe('hsts header missing analyzer', () => {
5252
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
5353
expect(vulnerabilities[0].evidence.value).to.be.equal('max-age=-100')
5454
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
55-
}, makeRequestWithXFordwardedProtoHeader)
55+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
5656

5757
testThatRequestHasVulnerability((req, res) => {
5858
res.setHeader('content-type', 'text/html')
@@ -61,7 +61,7 @@ describe('hsts header missing analyzer', () => {
6161
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
6262
expect(vulnerabilities[0].evidence.value).to.be.equal('max-age=-100; includeSubDomains')
6363
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
64-
}, makeRequestWithXFordwardedProtoHeader)
64+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
6565

6666
testThatRequestHasVulnerability((req, res) => {
6767
res.setHeader('content-type', 'text/html')
@@ -70,7 +70,7 @@ describe('hsts header missing analyzer', () => {
7070
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
7171
expect(vulnerabilities[0].evidence.value).to.be.equal('invalid')
7272
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
73-
}, makeRequestWithXFordwardedProtoHeader)
73+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
7474

7575
testThatRequestHasVulnerability((req, res) => {
7676
res.setHeader('content-type', ['text/html'])
@@ -79,7 +79,7 @@ describe('hsts header missing analyzer', () => {
7979
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
8080
expect(vulnerabilities[0].evidence.value).to.be.equal('invalid')
8181
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
82-
}, makeRequestWithXFordwardedProtoHeader)
82+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
8383

8484
testThatRequestHasVulnerability((req, res) => {
8585
res.setHeader('content-type', ['text/html'])
@@ -88,7 +88,7 @@ describe('hsts header missing analyzer', () => {
8888
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
8989
expect(vulnerabilities[0].evidence).to.be.undefined
9090
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
91-
}, makeRequestWithXFordwardedProtoHeader)
91+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
9292

9393
testThatRequestHasVulnerability((req, res) => {
9494
res.setHeader('content-type', ['text/html'])
@@ -97,7 +97,7 @@ describe('hsts header missing analyzer', () => {
9797
}, HSTS_HEADER_MISSING, 1, function (vulnerabilities) {
9898
expect(vulnerabilities[0].evidence.value).to.be.equal(JSON.stringify(['invalid1', 'invalid2']))
9999
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('HSTS_HEADER_MISSING:mocha'))
100-
}, makeRequestWithXFordwardedProtoHeader)
100+
}, makeRequestWithXFordwardedProtoHeader, undefined, false)
101101

102102
testThatRequestHasNoVulnerability((req, res) => {
103103
res.setHeader('content-type', 'application/json')

‎packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ const proxyquire = require('proxyquire')
66
describe('vulnerability-analyzer', () => {
77
const VULNERABLE_VALUE = 'VULNERABLE_VALUE'
88
const VULNERABILITY = 'VULNERABILITY'
9-
const VULNERABILITY_LOCATION_FROM_SOURCEMAP = { path: 'VULNERABILITY_LOCATION_FROM_SOURCEMAP', line: 42, column: 21 }
9+
const VULNERABILITY_LOCATION_FROM_SOURCEMAP = {
10+
path: 'VULNERABILITY_LOCATION_FROM_SOURCEMAP', line: 42, method: 'callFn'
11+
}
1012
const ANALYZER_TYPE = 'TEST_ANALYZER'
1113
const SPAN_ID = '123456'
1214

‎packages/dd-trace/test/appsec/iast/analyzers/xcontenttype-header-missing-analyzer.spec.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,23 @@ describe('xcontenttype header missing analyzer', () => {
1818
}, XCONTENTTYPE_HEADER_MISSING, 1, function (vulnerabilities) {
1919
expect(vulnerabilities[0].evidence).to.be.undefined
2020
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('XCONTENTTYPE_HEADER_MISSING:mocha'))
21-
})
21+
}, undefined, undefined, false)
2222

2323
testThatRequestHasVulnerability((req, res) => {
2424
res.setHeader('content-type', 'text/html;charset=utf-8')
2525
res.end('<html><body><h1>Test</h1></body></html>')
2626
}, XCONTENTTYPE_HEADER_MISSING, 1, function (vulnerabilities) {
2727
expect(vulnerabilities[0].evidence).to.be.undefined
2828
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('XCONTENTTYPE_HEADER_MISSING:mocha'))
29-
})
29+
}, undefined, undefined, false)
3030

3131
testThatRequestHasVulnerability((req, res) => {
3232
res.setHeader('content-type', 'application/xhtml+xml')
3333
res.end('<html><body><h1>Test</h1></body></html>')
3434
}, XCONTENTTYPE_HEADER_MISSING, 1, function (vulnerabilities) {
3535
expect(vulnerabilities[0].evidence).to.be.undefined
3636
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('XCONTENTTYPE_HEADER_MISSING:mocha'))
37-
})
37+
}, undefined, undefined, false)
3838

3939
testThatRequestHasVulnerability((req, res) => {
4040
res.setHeader('content-type', 'text/html')
@@ -43,7 +43,7 @@ describe('xcontenttype header missing analyzer', () => {
4343
}, XCONTENTTYPE_HEADER_MISSING, 1, function (vulnerabilities) {
4444
expect(vulnerabilities[0].evidence.value).to.be.equal('whatever')
4545
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('XCONTENTTYPE_HEADER_MISSING:mocha'))
46-
})
46+
}, undefined, undefined, false)
4747

4848
testThatRequestHasVulnerability((req, res) => {
4949
res.setHeader('content-type', ['text/html'])
@@ -52,7 +52,7 @@ describe('xcontenttype header missing analyzer', () => {
5252
}, XCONTENTTYPE_HEADER_MISSING, 1, function (vulnerabilities) {
5353
expect(vulnerabilities[0].evidence.value).to.be.equal('whatever')
5454
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('XCONTENTTYPE_HEADER_MISSING:mocha'))
55-
})
55+
}, undefined, undefined, false)
5656

5757
testThatRequestHasNoVulnerability((req, res) => {
5858
res.setHeader('content-type', 'application/json')

‎packages/dd-trace/test/appsec/iast/taint-tracking/sources/sql_row.sequelize.plugin.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('db sources with sequelize', () => {
3939

4040
res.end('OK')
4141
}, 'SQL_INJECTION', { occurrences: 1 }, null, null,
42-
'Should have SQL_INJECTION using the first row of the result')
42+
'Should have SQL_INJECTION using the first row of the result', false)
4343

4444
testThatRequestHasNoVulnerability(async (req, res) => {
4545
const result = await sequelize.query('SELECT * from examples')
@@ -82,7 +82,7 @@ describe('db sources with sequelize', () => {
8282

8383
res.end('OK')
8484
}, 'SQL_INJECTION', { occurrences: 1 }, null, null,
85-
'Should have SQL_INJECTION using the first row of the result')
85+
'Should have SQL_INJECTION using the first row of the result', false)
8686

8787
testThatRequestHasNoVulnerability(async (req, res) => {
8888
const examples = await Example.findAll()

‎packages/dd-trace/test/appsec/iast/utils.js

+49-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const fs = require('fs')
44
const os = require('os')
55
const path = require('path')
66
const { assert } = require('chai')
7+
const msgpack = require('@msgpack/msgpack')
78

89
const agent = require('../../plugins/agent')
910
const axios = require('axios')
@@ -152,7 +153,15 @@ function checkNoVulnerabilityInRequest (vulnerability, config, done, makeRequest
152153
}
153154
}
154155

155-
function checkVulnerabilityInRequest (vulnerability, occurrencesAndLocation, cb, makeRequest, config, done) {
156+
function checkVulnerabilityInRequest (
157+
vulnerability,
158+
occurrencesAndLocation,
159+
cb,
160+
makeRequest,
161+
config,
162+
done,
163+
matchLocation
164+
) {
156165
let location
157166
let occurrences = occurrencesAndLocation
158167
if (occurrencesAndLocation !== null && typeof occurrencesAndLocation === 'object') {
@@ -199,6 +208,12 @@ function checkVulnerabilityInRequest (vulnerability, occurrencesAndLocation, cb,
199208
}
200209
}
201210

211+
if (matchLocation) {
212+
const matchFound = locationHasMatchingFrame(span, vulnerability, vulnerabilitiesTrace.vulnerabilities)
213+
214+
assert.isTrue(matchFound)
215+
}
216+
202217
if (cb) {
203218
cb(vulnerabilitiesTrace.vulnerabilities.filter(v => v.type === vulnerability))
204219
}
@@ -254,11 +269,19 @@ function prepareTestServerForIast (description, tests, iastConfig) {
254269
return agent.close({ ritmReset: false })
255270
})
256271

257-
function testThatRequestHasVulnerability (fn, vulnerability, occurrences, cb, makeRequest, description) {
272+
function testThatRequestHasVulnerability (
273+
fn,
274+
vulnerability,
275+
occurrences,
276+
cb,
277+
makeRequest,
278+
description,
279+
matchLocation = true
280+
) {
258281
it(description || `should have ${vulnerability} vulnerability`, function (done) {
259282
this.timeout(5000)
260283
app = fn
261-
checkVulnerabilityInRequest(vulnerability, occurrences, cb, makeRequest, config, done)
284+
checkVulnerabilityInRequest(vulnerability, occurrences, cb, makeRequest, config, done, matchLocation)
262285
})
263286
}
264287

@@ -373,6 +396,29 @@ function prepareTestServerForIastInExpress (description, expressVersion, loadMid
373396
})
374397
}
375398

399+
function locationHasMatchingFrame (span, vulnerabilityType, vulnerabilities) {
400+
const stack = msgpack.decode(span.meta_struct['_dd.stack'])
401+
const matchingVulns = vulnerabilities.filter(vulnerability => vulnerability.type === vulnerabilityType)
402+
403+
for (const vulnerability of stack.vulnerability) {
404+
for (const frame of vulnerability.frames) {
405+
for (const { location } of matchingVulns) {
406+
if (
407+
frame.line === location.line &&
408+
frame.class_name === location.class &&
409+
frame.function === location.method &&
410+
frame.path === location.path &&
411+
!location.hasOwnProperty('column')
412+
) {
413+
return true
414+
}
415+
}
416+
}
417+
}
418+
419+
return false
420+
}
421+
376422
module.exports = {
377423
testOutsideRequestHasVulnerability,
378424
testInRequest,

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

+21-5
Original file line numberDiff line numberDiff line change
@@ -93,21 +93,37 @@ describe('Vulnerability formatter', () => {
9393
})
9494

9595
describe('toJson', () => {
96-
it('should filter out column property from location', () => {
96+
it('should format vulnerability correctly', () => {
9797
const vulnerabilities = [{
9898
type: 'test-vulnerability',
99+
hash: 123456,
100+
stackId: 1,
99101
evidence: {
100102
value: 'payload'
101103
},
102104
location: {
103105
path: 'path',
104-
line: 42,
105-
column: 3
106+
line: 42
106107
}
107108
}]
108109

109-
const json = vulnerabilityFormatter.toJson(vulnerabilities)
110-
expect(json.vulnerabilities[0].location.column).to.be.undefined
110+
const result = vulnerabilityFormatter.toJson(vulnerabilities)
111+
112+
expect(result).to.deep.equal({
113+
sources: [],
114+
vulnerabilities: [{
115+
type: 'test-vulnerability',
116+
hash: 123456,
117+
stackId: 1,
118+
evidence: {
119+
value: 'payload'
120+
},
121+
location: {
122+
path: 'path',
123+
line: 42
124+
}
125+
}]
126+
})
111127
})
112128
})
113129

0 commit comments

Comments
 (0)