Skip to content

Commit 949eccb

Browse files
IlyasShabiszegedi
authored andcommitted
Report stack trace in iast (#5055)
* Report stack trace in iast * fix stack trace tests * fix names * call site frames * fix path-line tests * use frames instead of call list * fix hardcoded-analyzers tests * clear tests * get original locations only if we can add vulnerability * add iast stacktrace variable * vulnerability reporter unit test with stack trace * maintain stack trace limit per request * dont report stack trace if we reach max by request * add use strict to utils test file
1 parent db8be83 commit 949eccb

22 files changed

+678
-326
lines changed

docs/test.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,10 @@ tracer.init({
136136
redactionEnabled: true,
137137
redactionNamePattern: 'password',
138138
redactionValuePattern: 'bearer',
139-
telemetryVerbosity: 'OFF'
139+
telemetryVerbosity: 'OFF',
140+
stackTrace: {
141+
enabled: true
142+
}
140143
}
141144
});
142145

index.d.ts

+11-1
Original file line numberDiff line numberDiff line change
@@ -2233,7 +2233,17 @@ declare namespace tracer {
22332233
/**
22342234
* Specifies the verbosity of the sent telemetry. Default 'INFORMATION'
22352235
*/
2236-
telemetryVerbosity?: string
2236+
telemetryVerbosity?: string,
2237+
2238+
/**
2239+
* Configuration for stack trace reporting
2240+
*/
2241+
stackTrace?: {
2242+
/** Whether to enable stack trace reporting.
2243+
* @default true
2244+
*/
2245+
enabled?: boolean,
2246+
}
22372247
}
22382248

22392249
export namespace llmobs {

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@ class CookieAnalyzer extends Analyzer {
5454
return super._checkOCE(context, value)
5555
}
5656

57-
_getLocation (value) {
57+
_getLocation (value, callSiteFrames) {
5858
if (!value) {
59-
return super._getLocation()
59+
return super._getLocation(value, callSiteFrames)
6060
}
6161

6262
if (value.location) {
6363
return value.location
6464
}
65-
const location = super._getLocation(value)
65+
const location = super._getLocation(value, callSiteFrames)
6666
value.location = location
6767
return location
6868
}

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

+41-24
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
'use strict'
22

33
const { storage } = require('../../../../../datadog-core')
4-
const { getFirstNonDDPathAndLine } = require('../path-line')
5-
const { addVulnerability } = require('../vulnerability-reporter')
6-
const { getIastContext } = require('../iast-context')
4+
const { getNonDDCallSiteFrames } = require('../path-line')
5+
const { getIastContext, getIastStackTraceId } = require('../iast-context')
76
const overheadController = require('../overhead-controller')
87
const { SinkIastPlugin } = require('../iast-plugin')
9-
const { getOriginalPathAndLineFromSourceMap } = require('../taint-tracking/rewriter')
8+
const {
9+
addVulnerability,
10+
getVulnerabilityCallSiteFrames,
11+
replaceCallSiteFromSourceMap
12+
} = require('../vulnerability-reporter')
1013

1114
class Analyzer extends SinkIastPlugin {
1215
constructor (type) {
@@ -28,12 +31,24 @@ class Analyzer extends SinkIastPlugin {
2831
}
2932

3033
_reportEvidence (value, context, evidence) {
31-
const location = this._getLocation(value)
34+
const callSiteFrames = getVulnerabilityCallSiteFrames()
35+
const nonDDCallSiteFrames = getNonDDCallSiteFrames(callSiteFrames, this._getExcludedPaths())
36+
37+
const location = this._getLocation(value, nonDDCallSiteFrames)
38+
3239
if (!this._isExcluded(location)) {
33-
const locationSourceMap = this._replaceLocationFromSourceMap(location)
40+
const originalLocation = this._getOriginalLocation(location)
3441
const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId()
35-
const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap)
36-
addVulnerability(context, vulnerability)
42+
const stackId = getIastStackTraceId(context)
43+
const vulnerability = this._createVulnerability(
44+
this._type,
45+
evidence,
46+
spanId,
47+
originalLocation,
48+
stackId
49+
)
50+
51+
addVulnerability(context, vulnerability, nonDDCallSiteFrames)
3752
}
3853
}
3954

@@ -49,24 +64,25 @@ class Analyzer extends SinkIastPlugin {
4964
return { value }
5065
}
5166

52-
_getLocation () {
53-
return getFirstNonDDPathAndLine(this._getExcludedPaths())
67+
_getLocation (value, callSiteFrames) {
68+
return callSiteFrames[0]
5469
}
5570

56-
_replaceLocationFromSourceMap (location) {
57-
if (location) {
58-
const { path, line, column } = getOriginalPathAndLineFromSourceMap(location)
59-
if (path) {
60-
location.path = path
61-
}
62-
if (line) {
63-
location.line = line
64-
}
65-
if (column) {
66-
location.column = column
67-
}
71+
_getOriginalLocation (location) {
72+
const locationFromSourceMap = replaceCallSiteFromSourceMap(location)
73+
const originalLocation = {}
74+
75+
if (locationFromSourceMap?.path) {
76+
originalLocation.path = locationFromSourceMap.path
77+
}
78+
if (locationFromSourceMap?.line) {
79+
originalLocation.line = locationFromSourceMap.line
6880
}
69-
return location
81+
if (locationFromSourceMap?.column) {
82+
originalLocation.column = locationFromSourceMap.column
83+
}
84+
85+
return originalLocation
7086
}
7187

7288
_getExcludedPaths () {}
@@ -102,12 +118,13 @@ class Analyzer extends SinkIastPlugin {
102118
return overheadController.hasQuota(overheadController.OPERATIONS.REPORT_VULNERABILITY, context)
103119
}
104120

105-
_createVulnerability (type, evidence, spanId, location) {
121+
_createVulnerability (type, evidence, spanId, location, stackId) {
106122
if (type && evidence) {
107123
const _spanId = spanId || 0
108124
return {
109125
type,
110126
evidence,
127+
stackId,
111128
location: {
112129
spanId: _spanId,
113130
...location

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

+12
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,17 @@ function getIastContext (store, topContext) {
99
return iastContext
1010
}
1111

12+
function getIastStackTraceId (iastContext) {
13+
if (!iastContext) return 0
14+
15+
if (!iastContext.stackTraceId) {
16+
iastContext.stackTraceId = 0
17+
}
18+
19+
iastContext.stackTraceId += 1
20+
return iastContext.stackTraceId
21+
}
22+
1223
/* TODO Fix storage problem when the close event is called without
1324
finish event to remove `topContext` references
1425
We have to save the context in two places, because
@@ -51,6 +62,7 @@ module.exports = {
5162
getIastContext,
5263
saveIastContext,
5364
cleanIastContext,
65+
getIastStackTraceId,
5466
IAST_CONTEXT_KEY,
5567
IAST_TRANSACTION_ID
5668
}

packages/dd-trace/src/appsec/iast/path-line.js

+19-23
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,10 @@
33
const path = require('path')
44
const process = require('process')
55
const { calculateDDBasePath } = require('../../util')
6-
const { getCallSiteList } = require('../stack_trace')
76
const pathLine = {
8-
getFirstNonDDPathAndLine,
97
getNodeModulesPaths,
108
getRelativePath,
11-
getFirstNonDDPathAndLineFromCallsites, // Exported only for test purposes
9+
getNonDDCallSiteFrames,
1210
calculateDDBasePath, // Exported only for test purposes
1311
ddBasePath: calculateDDBasePath(__dirname) // Only for test purposes
1412
}
@@ -25,31 +23,33 @@ const EXCLUDED_PATH_PREFIXES = [
2523
'async_hooks'
2624
]
2725

28-
function getFirstNonDDPathAndLineFromCallsites (callsites, externallyExcludedPaths) {
29-
if (callsites) {
30-
for (let i = 0; i < callsites.length; i++) {
31-
const callsite = callsites[i]
32-
const filepath = callsite.getFileName()
33-
if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) {
34-
return {
35-
path: getRelativePath(filepath),
36-
line: callsite.getLineNumber(),
37-
column: callsite.getColumnNumber(),
38-
isInternal: !path.isAbsolute(filepath)
39-
}
40-
}
26+
function getNonDDCallSiteFrames (callSiteFrames, externallyExcludedPaths) {
27+
if (!callSiteFrames) {
28+
return []
29+
}
30+
31+
const result = []
32+
33+
for (const callsite of callSiteFrames) {
34+
const filepath = callsite.file
35+
if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) {
36+
callsite.path = getRelativePath(filepath)
37+
callsite.isInternal = !path.isAbsolute(filepath)
38+
39+
result.push(callsite)
4140
}
4241
}
43-
return null
42+
43+
return result
4444
}
4545

4646
function getRelativePath (filepath) {
4747
return path.relative(process.cwd(), filepath)
4848
}
4949

5050
function isExcluded (callsite, externallyExcludedPaths) {
51-
if (callsite.isNative()) return true
52-
const filename = callsite.getFileName()
51+
if (callsite.isNative) return true
52+
const filename = callsite.file
5353
if (!filename) {
5454
return true
5555
}
@@ -73,10 +73,6 @@ function isExcluded (callsite, externallyExcludedPaths) {
7373
return false
7474
}
7575

76-
function getFirstNonDDPathAndLine (externallyExcludedPaths) {
77-
return getFirstNonDDPathAndLineFromCallsites(getCallSiteList(), externallyExcludedPaths)
78-
}
79-
8076
function getNodeModulesPaths (...paths) {
8177
const nodeModulesPaths = []
8278

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

+1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class VulnerabilityFormatter {
8484
const formattedVulnerability = {
8585
type: vulnerability.type,
8686
hash: vulnerability.hash,
87+
stackId: vulnerability.stackId,
8788
evidence: this.formatEvidence(vulnerability.type, vulnerability.evidence, sourcesIndexes, sources),
8889
location: {
8990
spanId: vulnerability.location.spanId

0 commit comments

Comments
 (0)