From f6edc6e09f272956611b5f6967b81d990aa80e7b Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 23 Dec 2024 15:44:24 +0100 Subject: [PATCH 01/14] Report stack trace in iast --- .../appsec/iast/analyzers/cookie-analyzer.js | 6 ++-- .../iast/analyzers/vulnerability-analyzer.js | 12 ++++--- .../dd-trace/src/appsec/iast/iast-context.js | 12 +++++++ .../dd-trace/src/appsec/iast/path-line.js | 5 ++- .../iast/vulnerabilities-formatter/index.js | 1 + .../src/appsec/iast/vulnerability-reporter.js | 28 +++++++++++++++-- packages/dd-trace/src/appsec/rasp/utils.js | 12 +++++-- packages/dd-trace/src/appsec/stack_trace.js | 31 ++++++++++++------- packages/dd-trace/test/appsec/iast/utils.js | 16 ++++++++++ 9 files changed, 96 insertions(+), 27 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js index a898a0a379c..75117d78bde 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js @@ -54,15 +54,15 @@ class CookieAnalyzer extends Analyzer { return super._checkOCE(context, value) } - _getLocation (value) { + _getLocation (value, callSiteList) { if (!value) { - return super._getLocation() + return super._getLocation(value, callSiteList) } if (value.location) { return value.location } - const location = super._getLocation(value) + const location = super._getLocation(value, callSiteList) value.location = location return location } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index f79e7a44f71..e66d8c20e57 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -2,7 +2,7 @@ const { storage } = require('../../../../../datadog-core') const { getFirstNonDDPathAndLine } = require('../path-line') -const { addVulnerability } = require('../vulnerability-reporter') +const { addVulnerability, getVulnerabilityCallSiteList } = require('../vulnerability-reporter') const { getIastContext } = require('../iast-context') const overheadController = require('../overhead-controller') const { SinkIastPlugin } = require('../iast-plugin') @@ -28,12 +28,14 @@ class Analyzer extends SinkIastPlugin { } _reportEvidence (value, context, evidence) { - const location = this._getLocation(value) + const callSiteList = getVulnerabilityCallSiteList() + const location = this._getLocation(value, callSiteList) + if (!this._isExcluded(location)) { const locationSourceMap = this._replaceLocationFromSourceMap(location) const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId() const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap) - addVulnerability(context, vulnerability) + addVulnerability(context, vulnerability, callSiteList) } } @@ -49,8 +51,8 @@ class Analyzer extends SinkIastPlugin { return { value } } - _getLocation () { - return getFirstNonDDPathAndLine(this._getExcludedPaths()) + _getLocation (value, callSiteList) { + return getFirstNonDDPathAndLine(callSiteList, this._getExcludedPaths()) } _replaceLocationFromSourceMap (location) { diff --git a/packages/dd-trace/src/appsec/iast/iast-context.js b/packages/dd-trace/src/appsec/iast/iast-context.js index 6d697dcf978..eb98d81e17c 100644 --- a/packages/dd-trace/src/appsec/iast/iast-context.js +++ b/packages/dd-trace/src/appsec/iast/iast-context.js @@ -9,6 +9,17 @@ function getIastContext (store, topContext) { return iastContext } +function getIastStackTraceId (iastContext) { + if (!iastContext) return 0 + + if (!iastContext.iastStackTraceId) { + iastContext.iastStackTraceId = 0 + } + + iastContext.iastStackTraceId += 1 + return iastContext.iastStackTraceId +} + /* TODO Fix storage problem when the close event is called without finish event to remove `topContext` references We have to save the context in two places, because @@ -51,6 +62,7 @@ module.exports = { getIastContext, saveIastContext, cleanIastContext, + getIastStackTraceId, IAST_CONTEXT_KEY, IAST_TRANSACTION_ID } diff --git a/packages/dd-trace/src/appsec/iast/path-line.js b/packages/dd-trace/src/appsec/iast/path-line.js index bf7c3eb2d84..3c034aad8f2 100644 --- a/packages/dd-trace/src/appsec/iast/path-line.js +++ b/packages/dd-trace/src/appsec/iast/path-line.js @@ -3,7 +3,6 @@ const path = require('path') const process = require('process') const { calculateDDBasePath } = require('../../util') -const { getCallSiteList } = require('../stack_trace') const pathLine = { getFirstNonDDPathAndLine, getNodeModulesPaths, @@ -73,8 +72,8 @@ function isExcluded (callsite, externallyExcludedPaths) { return false } -function getFirstNonDDPathAndLine (externallyExcludedPaths) { - return getFirstNonDDPathAndLineFromCallsites(getCallSiteList(), externallyExcludedPaths) +function getFirstNonDDPathAndLine (callSiteList, externallyExcludedPaths) { + return getFirstNonDDPathAndLineFromCallsites(callSiteList, externallyExcludedPaths) } function getNodeModulesPaths (...paths) { diff --git a/packages/dd-trace/src/appsec/iast/vulnerabilities-formatter/index.js b/packages/dd-trace/src/appsec/iast/vulnerabilities-formatter/index.js index d704743dde4..88af720a285 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerabilities-formatter/index.js +++ b/packages/dd-trace/src/appsec/iast/vulnerabilities-formatter/index.js @@ -84,6 +84,7 @@ class VulnerabilityFormatter { const formattedVulnerability = { type: vulnerability.type, hash: vulnerability.hash, + stackId: vulnerability.stackId, evidence: this.formatEvidence(vulnerability.type, vulnerability.evidence, sourcesIndexes, sources), location: { spanId: vulnerability.location.spanId diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 05aea14cf02..961dd4a2cbb 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -6,6 +6,8 @@ const { IAST_ENABLED_TAG_KEY, IAST_JSON_TAG_KEY } = require('./tags') const standalone = require('../standalone') const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') const { keepTrace } = require('../../priority_sampler') +const { reportStackTrace, getCallSiteList, STACK_TRACE_NAMESPACES } = require('../stack_trace') +const { getIastStackTraceId } = require('./iast-context') const VULNERABILITIES_KEY = 'vulnerabilities' const VULNERABILITY_HASHES_MAX_SIZE = 1000 @@ -15,8 +17,10 @@ const RESET_VULNERABILITY_CACHE_INTERVAL = 60 * 60 * 1000 // 1 hour let tracer let resetVulnerabilityCacheTimer let deduplicationEnabled = true +let stackTraceMaxDepth +let maxStackTraces -function addVulnerability (iastContext, vulnerability) { +function addVulnerability (iastContext, vulnerability, callSiteList) { if (vulnerability?.evidence && vulnerability?.type && vulnerability?.location) { if (deduplicationEnabled && isDuplicatedVulnerability(vulnerability)) return @@ -41,6 +45,18 @@ function addVulnerability (iastContext, vulnerability) { keepTrace(span, SAMPLING_MECHANISM_APPSEC) standalone.sample(span) + const stackId = getIastStackTraceId(iastContext) + vulnerability.stackId = stackId + + reportStackTrace( + iastContext?.rootSpan, + stackId, + stackTraceMaxDepth, + maxStackTraces, + callSiteList, + STACK_TRACE_NAMESPACES.IAST + ) + if (iastContext?.rootSpan) { iastContext[VULNERABILITIES_KEY] = iastContext[VULNERABILITIES_KEY] || [] iastContext[VULNERABILITIES_KEY].push(vulnerability) @@ -91,11 +107,18 @@ function stopClearCacheTimer () { } function isDuplicatedVulnerability (vulnerability) { - return VULNERABILITY_HASHES.get(`${vulnerability.type}${vulnerability.hash}`) + return VULNERABILITY_HASHES.has(`${vulnerability.type}${vulnerability.hash}`) +} + +function getVulnerabilityCallSiteList () { + return getCallSiteList(stackTraceMaxDepth) } function start (config, _tracer) { deduplicationEnabled = config.iast.deduplicationEnabled + stackTraceMaxDepth = config.appsec.stackTrace.maxDepth + maxStackTraces = config.appsec.stackTrace.maxStackTraces + vulnerabilitiesFormatter.setRedactVulnerabilities( config.iast.redactionEnabled, config.iast.redactionNamePattern, @@ -114,6 +137,7 @@ function stop () { module.exports = { addVulnerability, sendVulnerabilities, + getVulnerabilityCallSiteList, clearCache, start, stop diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index a454a71b8c6..edca8a9ebd4 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -1,7 +1,7 @@ 'use strict' const web = require('../../plugins/util/web') -const { reportStackTrace } = require('../stack_trace') +const { getCallSiteList, reportStackTrace } = require('../stack_trace') const { getBlockingAction } = require('../blocking') const log = require('../../log') @@ -30,13 +30,19 @@ class DatadogRaspAbortError extends Error { function handleResult (actions, req, res, abortController, config) { const generateStackTraceAction = actions?.generate_stack + + const { maxDepth, maxStackTraces } = config.appsec.stackTrace + + const callSiteList = getCallSiteList(maxDepth) + if (generateStackTraceAction && config.appsec.stackTrace.enabled) { const rootSpan = web.root(req) reportStackTrace( rootSpan, generateStackTraceAction.stack_id, - config.appsec.stackTrace.maxDepth, - config.appsec.stackTrace.maxStackTraces + maxDepth, + maxStackTraces, + callSiteList ) } diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index ea49ed1e877..dd9a0f4cfb5 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -6,11 +6,22 @@ const ddBasePath = calculateDDBasePath(__dirname) const LIBRARY_FRAMES_BUFFER = 20 +const STACK_TRACE_NAMESPACES = { + RASP: 'exploit', + IAST: 'vulnerability' +} + function getCallSiteList (maxDepth = 100) { + if (maxDepth < 1) { + maxDepth = Infinity + } + const previousPrepareStackTrace = Error.prepareStackTrace const previousStackTraceLimit = Error.stackTraceLimit let callsiteList - Error.stackTraceLimit = maxDepth + // Since some frames will be discarded because they come from tracer codebase, a buffer is added + // to the limit in order to get as close as `maxDepth` number of frames. + Error.stackTraceLimit = maxDepth + LIBRARY_FRAMES_BUFFER try { Error.prepareStackTrace = function (_, callsites) { @@ -52,14 +63,11 @@ function getFramesForMetaStruct (callSiteList, maxDepth = 32) { return indexedFrames } -function reportStackTrace (rootSpan, stackId, maxDepth, maxStackTraces, callSiteListGetter = getCallSiteList) { +function reportStackTrace ( + rootSpan, stackId, maxDepth, maxStackTraces, callSiteList, namespace = STACK_TRACE_NAMESPACES.RASP) { if (!rootSpan) return - if (maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.exploit?.length ?? 0) < maxStackTraces) { - // Since some frames will be discarded because they come from tracer codebase, a buffer is added - // to the limit in order to get as close as `maxDepth` number of frames. - if (maxDepth < 1) maxDepth = Infinity - const callSiteList = callSiteListGetter(maxDepth + LIBRARY_FRAMES_BUFFER) + if (maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.[namespace]?.length ?? 0) < maxStackTraces) { if (!Array.isArray(callSiteList)) return if (!rootSpan.meta_struct) { @@ -70,13 +78,13 @@ function reportStackTrace (rootSpan, stackId, maxDepth, maxStackTraces, callSite rootSpan.meta_struct['_dd.stack'] = {} } - if (!rootSpan.meta_struct['_dd.stack'].exploit) { - rootSpan.meta_struct['_dd.stack'].exploit = [] + if (!rootSpan.meta_struct['_dd.stack'][namespace]) { + rootSpan.meta_struct['_dd.stack'][namespace] = [] } const frames = getFramesForMetaStruct(callSiteList, maxDepth) - rootSpan.meta_struct['_dd.stack'].exploit.push({ + rootSpan.meta_struct['_dd.stack'][namespace].push({ id: stackId, language: 'nodejs', frames @@ -86,5 +94,6 @@ function reportStackTrace (rootSpan, stackId, maxDepth, maxStackTraces, callSite module.exports = { getCallSiteList, - reportStackTrace + reportStackTrace, + STACK_TRACE_NAMESPACES } diff --git a/packages/dd-trace/test/appsec/iast/utils.js b/packages/dd-trace/test/appsec/iast/utils.js index 01274dd954e..ee6abe13cba 100644 --- a/packages/dd-trace/test/appsec/iast/utils.js +++ b/packages/dd-trace/test/appsec/iast/utils.js @@ -3,6 +3,7 @@ const fs = require('fs') const os = require('os') const path = require('path') +const { assert } = require('chai') const agent = require('../../plugins/agent') const axios = require('axios') @@ -10,6 +11,17 @@ const iast = require('../../../src/appsec/iast') const Config = require('../../../src/config') const vulnerabilityReporter = require('../../../src/appsec/iast/vulnerability-reporter') +function getWebSpan (traces) { + for (const trace of traces) { + for (const span of trace) { + if (span.type === 'web') { + return span + } + } + } + throw new Error('web span not found') +} + function testInRequest (app, tests) { let http let listener @@ -161,6 +173,10 @@ function checkVulnerabilityInRequest (vulnerability, occurrencesAndLocation, cb, .use(traces => { expect(traces[0][0].metrics['_dd.iast.enabled']).to.be.equal(1) expect(traces[0][0].meta).to.have.property('_dd.iast.json') + + const span = getWebSpan(traces) + assert.property(span.meta_struct, '_dd.stack') + const vulnerabilitiesTrace = JSON.parse(traces[0][0].meta['_dd.iast.json']) expect(vulnerabilitiesTrace).to.not.be.null const vulnerabilitiesCount = new Map() From 3cc25afbfae10163b2a8dfe782b75cc6d1ff52dc Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 23 Dec 2024 17:39:27 +0100 Subject: [PATCH 02/14] fix stack trace tests --- .../iast/analyzers/vulnerability-analyzer.js | 10 ++-- .../src/appsec/iast/vulnerability-reporter.js | 6 +- packages/dd-trace/src/appsec/rasp/utils.js | 4 +- packages/dd-trace/src/appsec/stack_trace.js | 5 +- .../analyzers/vulnerability-analyzer.spec.js | 10 +++- .../test/appsec/iast/path-line.spec.js | 6 +- .../iast/vulnerability-reporter.spec.js | 55 ++++++++++++++++++- .../dd-trace/test/appsec/rasp/utils.spec.js | 5 +- .../dd-trace/test/appsec/stack_trace.spec.js | 26 ++++----- 9 files changed, 91 insertions(+), 36 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index e66d8c20e57..8ff4d13a6a4 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -3,7 +3,7 @@ const { storage } = require('../../../../../datadog-core') const { getFirstNonDDPathAndLine } = require('../path-line') const { addVulnerability, getVulnerabilityCallSiteList } = require('../vulnerability-reporter') -const { getIastContext } = require('../iast-context') +const { getIastContext, getIastStackTraceId } = require('../iast-context') const overheadController = require('../overhead-controller') const { SinkIastPlugin } = require('../iast-plugin') const { getOriginalPathAndLineFromSourceMap } = require('../taint-tracking/rewriter') @@ -34,8 +34,9 @@ class Analyzer extends SinkIastPlugin { if (!this._isExcluded(location)) { const locationSourceMap = this._replaceLocationFromSourceMap(location) const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId() - const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap) - addVulnerability(context, vulnerability, callSiteList) + const stackId = getIastStackTraceId(context) + const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap, stackId) + addVulnerability(context, vulnerability, callSiteList, stackId) } } @@ -104,12 +105,13 @@ class Analyzer extends SinkIastPlugin { return overheadController.hasQuota(overheadController.OPERATIONS.REPORT_VULNERABILITY, context) } - _createVulnerability (type, evidence, spanId, location) { + _createVulnerability (type, evidence, spanId, location, stackId) { if (type && evidence) { const _spanId = spanId || 0 return { type, evidence, + stackId, location: { spanId: _spanId, ...location diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 961dd4a2cbb..711815e2da5 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -7,7 +7,6 @@ const standalone = require('../standalone') const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') const { keepTrace } = require('../../priority_sampler') const { reportStackTrace, getCallSiteList, STACK_TRACE_NAMESPACES } = require('../stack_trace') -const { getIastStackTraceId } = require('./iast-context') const VULNERABILITIES_KEY = 'vulnerabilities' const VULNERABILITY_HASHES_MAX_SIZE = 1000 @@ -20,7 +19,7 @@ let deduplicationEnabled = true let stackTraceMaxDepth let maxStackTraces -function addVulnerability (iastContext, vulnerability, callSiteList) { +function addVulnerability (iastContext, vulnerability, callSiteList, stackId) { if (vulnerability?.evidence && vulnerability?.type && vulnerability?.location) { if (deduplicationEnabled && isDuplicatedVulnerability(vulnerability)) return @@ -45,9 +44,6 @@ function addVulnerability (iastContext, vulnerability, callSiteList) { keepTrace(span, SAMPLING_MECHANISM_APPSEC) standalone.sample(span) - const stackId = getIastStackTraceId(iastContext) - vulnerability.stackId = stackId - reportStackTrace( iastContext?.rootSpan, stackId, diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index edca8a9ebd4..9edc7c70092 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -31,11 +31,11 @@ class DatadogRaspAbortError extends Error { function handleResult (actions, req, res, abortController, config) { const generateStackTraceAction = actions?.generate_stack - const { maxDepth, maxStackTraces } = config.appsec.stackTrace + const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace const callSiteList = getCallSiteList(maxDepth) - if (generateStackTraceAction && config.appsec.stackTrace.enabled) { + if (generateStackTraceAction && enabled) { const rootSpan = web.root(req) reportStackTrace( rootSpan, diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index dd9a0f4cfb5..709783d2985 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -12,9 +12,7 @@ const STACK_TRACE_NAMESPACES = { } function getCallSiteList (maxDepth = 100) { - if (maxDepth < 1) { - maxDepth = Infinity - } + if (maxDepth < 1) maxDepth = Infinity const previousPrepareStackTrace = Error.prepareStackTrace const previousStackTraceLimit = Error.stackTraceLimit @@ -68,6 +66,7 @@ function reportStackTrace ( if (!rootSpan) return if (maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.[namespace]?.length ?? 0) < maxStackTraces) { + if (maxDepth < 1) maxDepth = Infinity if (!Array.isArray(callSiteList)) return if (!rootSpan.meta_struct) { diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index b47fb95b81b..2d882844c0b 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -21,7 +21,8 @@ describe('vulnerability-analyzer', () => { beforeEach(() => { vulnerabilityReporter = { createVulnerability: sinon.stub().returns(VULNERABILITY), - addVulnerability: sinon.stub() + addVulnerability: sinon.stub(), + getVulnerabilityCallSiteList: sinon.stub().returns([]) } pathLine = { getFirstNonDDPathAndLine: sinon.stub().returns(VULNERABILITY_LOCATION) @@ -116,10 +117,11 @@ describe('vulnerability-analyzer', () => { } } vulnerabilityAnalyzer._report(VULNERABLE_VALUE, context) - expect(vulnerabilityReporter.addVulnerability).to.have.been.calledOnceWithExactly( + expect(vulnerabilityReporter.addVulnerability).to.have.been.calledOnceWith( context, { type: 'TEST_ANALYZER', + stackId: 1, evidence: { value: 'VULNERABLE_VALUE' }, @@ -129,7 +131,9 @@ describe('vulnerability-analyzer', () => { line: 42 }, hash: 5975567724 - } + }, + [], + 1 ) }) diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js index 11905bcb880..aa65c0ff30d 100644 --- a/packages/dd-trace/test/appsec/iast/path-line.spec.js +++ b/packages/dd-trace/test/appsec/iast/path-line.spec.js @@ -52,7 +52,11 @@ describe('path-line', function () { describe('getFirstNonDDPathAndLine', () => { it('call does not fail', () => { - const obj = pathLine.getFirstNonDDPathAndLine() + const PROJECT_PATH = path.join(rootPath, 'project-path') + const DD_BASE_PATH = path.join(PROJECT_PATH, 'node_modules', 'dd-trace') + const callSiteList = [] + callSiteList.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) + const obj = pathLine.getFirstNonDDPathAndLine(callSiteList, false) expect(obj).to.not.be.null }) }) diff --git a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js index 2ebe646a2d8..3798882c727 100644 --- a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js +++ b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js @@ -107,6 +107,13 @@ describe('vulnerability-reporter', () => { start({ iast: { deduplicationEnabled: true + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } }, fakeTracer) }) @@ -119,7 +126,7 @@ describe('vulnerability-reporter', () => { it('should create span on the fly', () => { const vulnerability = vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, undefined, - { path: 'filename.js', line: 73 }) + { path: 'filename.js', line: 73 }, 1) addVulnerability(undefined, vulnerability) expect(fakeTracer.startSpan).to.have.been.calledOnceWithExactly('vulnerability', { type: 'vulnerability' }) expect(onTheFlySpan.addTags.firstCall).to.have.been.calledWithExactly({ @@ -127,7 +134,7 @@ describe('vulnerability-reporter', () => { }) expect(onTheFlySpan.addTags.secondCall).to.have.been.calledWithExactly({ '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512655,' + - '"evidence":{"value":"sha1"},"location":{"spanId":42,"path":"filename.js","line":73}}]}' + '"stackId":1,"evidence":{"value":"sha1"},"location":{"spanId":42,"path":"filename.js","line":73}}]}' }) expect(prioritySampler.setPriority) .to.have.been.calledOnceWithExactly(onTheFlySpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) @@ -162,6 +169,13 @@ describe('vulnerability-reporter', () => { start({ iast: { deduplicationEnabled: true + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } }) }) @@ -386,6 +400,13 @@ describe('vulnerability-reporter', () => { start({ iast: { deduplicationEnabled: false + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } }) const iastContext = { rootSpan: span } @@ -477,7 +498,7 @@ describe('vulnerability-reporter', () => { const MAX = 1000 const vulnerabilityToRepeatInTheNext = vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888, - { path: 'filename.js', line: 0 }) + { path: 'filename.js', line: 0 }, 1) addVulnerability(iastContext, vulnerabilityToRepeatInTheNext) for (let i = 1; i <= MAX; i++) { addVulnerability(iastContext, @@ -497,6 +518,13 @@ describe('vulnerability-reporter', () => { const config = { iast: { deduplicationEnabled: true + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } } start(config) @@ -507,6 +535,13 @@ describe('vulnerability-reporter', () => { const config = { iast: { deduplicationEnabled: false + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } } start(config) @@ -517,6 +552,13 @@ describe('vulnerability-reporter', () => { const config = { iast: { deduplicationEnabled: true + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } } start(config) @@ -542,6 +584,13 @@ describe('vulnerability-reporter', () => { redactionEnabled: true, redactionNamePattern: null, redactionValuePattern: null + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } } } start(config) diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js index 255f498a117..061c3e3c573 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.spec.js +++ b/packages/dd-trace/test/appsec/rasp/utils.spec.js @@ -11,7 +11,8 @@ describe('RASP - utils.js', () => { } stackTrace = { - reportStackTrace: sinon.stub() + reportStackTrace: sinon.stub(), + getCallSiteList: sinon.stub().returns({}) } utils = proxyquire('../../../src/appsec/rasp/utils', { @@ -44,7 +45,7 @@ describe('RASP - utils.js', () => { web.root.returns(rootSpan) utils.handleResult(result, req, undefined, undefined, config) - sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2) + sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2, {}) }) it('should not report stack trace when no action is present in waf result', () => { diff --git a/packages/dd-trace/test/appsec/stack_trace.spec.js b/packages/dd-trace/test/appsec/stack_trace.spec.js index 1ac2ca4db5e..c66e4643ba3 100644 --- a/packages/dd-trace/test/appsec/stack_trace.spec.js +++ b/packages/dd-trace/test/appsec/stack_trace.spec.js @@ -62,7 +62,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 const maxStackTraces = 2 - reportStackTrace(rootSpan, stackId, maxDepth, maxStackTraces, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, maxStackTraces, callSiteList) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -84,7 +84,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 try { - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) } catch (e) { assert.fail() } @@ -105,7 +105,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].language, 'nodejs') @@ -131,7 +131,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].language, 'nodejs') @@ -161,7 +161,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[1].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[1].language, 'nodejs') @@ -181,7 +181,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 2) assert.property(rootSpan.meta_struct, 'another_tag') @@ -199,7 +199,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - reportStackTrace(rootSpan, stackId, maxDepth, 0, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 0, callSiteList) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 3) assert.property(rootSpan.meta_struct, 'another_tag') @@ -217,7 +217,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - reportStackTrace(rootSpan, stackId, maxDepth, -1, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, -1, callSiteList) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 3) assert.property(rootSpan.meta_struct, 'another_tag') @@ -232,7 +232,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 const maxStackTraces = 2 - reportStackTrace(rootSpan, stackId, maxDepth, maxStackTraces, () => undefined) + reportStackTrace(rootSpan, stackId, maxDepth, maxStackTraces, undefined) assert.property(rootSpan.meta_struct, 'another_tag') assert.notProperty(rootSpan.meta_struct, '_dd.stack') }) @@ -264,7 +264,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -309,7 +309,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteListWithLibraryFrames) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteListWithLibraryFrames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -329,7 +329,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -349,7 +349,7 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, () => callSiteList) + reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) From 0a9cc8d6e0643a499d3497116083602621ad5172 Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 24 Dec 2024 09:38:27 +0100 Subject: [PATCH 03/14] fix names --- packages/dd-trace/src/appsec/iast/iast-context.js | 8 ++++---- .../dd-trace/src/appsec/iast/vulnerability-reporter.js | 2 +- packages/dd-trace/src/appsec/rasp/utils.js | 4 ++-- .../appsec/iast/analyzers/vulnerability-analyzer.spec.js | 2 +- packages/dd-trace/test/appsec/rasp/utils.spec.js | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/iast-context.js b/packages/dd-trace/src/appsec/iast/iast-context.js index eb98d81e17c..77c757fff8a 100644 --- a/packages/dd-trace/src/appsec/iast/iast-context.js +++ b/packages/dd-trace/src/appsec/iast/iast-context.js @@ -12,12 +12,12 @@ function getIastContext (store, topContext) { function getIastStackTraceId (iastContext) { if (!iastContext) return 0 - if (!iastContext.iastStackTraceId) { - iastContext.iastStackTraceId = 0 + if (!iastContext.stackTraceId) { + iastContext.stackTraceId = 0 } - iastContext.iastStackTraceId += 1 - return iastContext.iastStackTraceId + iastContext.stackTraceId += 1 + return iastContext.stackTraceId } /* TODO Fix storage problem when the close event is called without diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 711815e2da5..264e7f916e6 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -103,7 +103,7 @@ function stopClearCacheTimer () { } function isDuplicatedVulnerability (vulnerability) { - return VULNERABILITY_HASHES.has(`${vulnerability.type}${vulnerability.hash}`) + return VULNERABILITY_HASHES.get(`${vulnerability.type}${vulnerability.hash}`) } function getVulnerabilityCallSiteList () { diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index 9edc7c70092..4908d8c8687 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -33,9 +33,9 @@ function handleResult (actions, req, res, abortController, config) { const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace - const callSiteList = getCallSiteList(maxDepth) - if (generateStackTraceAction && enabled) { + const callSiteList = getCallSiteList(maxDepth) + const rootSpan = web.root(req) reportStackTrace( rootSpan, diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index 2d882844c0b..3a4ad3a9af6 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -117,7 +117,7 @@ describe('vulnerability-analyzer', () => { } } vulnerabilityAnalyzer._report(VULNERABLE_VALUE, context) - expect(vulnerabilityReporter.addVulnerability).to.have.been.calledOnceWith( + expect(vulnerabilityReporter.addVulnerability).to.have.been.calledOnceWithExactly( context, { type: 'TEST_ANALYZER', diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js index 061c3e3c573..2f6918dd0ab 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.spec.js +++ b/packages/dd-trace/test/appsec/rasp/utils.spec.js @@ -12,7 +12,7 @@ describe('RASP - utils.js', () => { stackTrace = { reportStackTrace: sinon.stub(), - getCallSiteList: sinon.stub().returns({}) + getCallSiteList: sinon.stub().returns([]) } utils = proxyquire('../../../src/appsec/rasp/utils', { @@ -45,7 +45,7 @@ describe('RASP - utils.js', () => { web.root.returns(rootSpan) utils.handleResult(result, req, undefined, undefined, config) - sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2, {}) + sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2, []) }) it('should not report stack trace when no action is present in waf result', () => { From de09037964d4b9eddaf0cb6c0256919175a39766 Mon Sep 17 00:00:00 2001 From: ishabi Date: Fri, 10 Jan 2025 16:29:35 +0100 Subject: [PATCH 04/14] call site frames --- .../iast/analyzers/vulnerability-analyzer.js | 49 +++-- .../dd-trace/src/appsec/iast/path-line.js | 39 ++-- packages/dd-trace/src/appsec/stack_trace.js | 9 +- .../analyzers/vulnerability-analyzer.spec.js | 6 +- .../test/appsec/iast/path-line.spec.js | 174 +++++++++--------- .../dd-trace/test/appsec/stack_trace.spec.js | 51 +++-- 6 files changed, 180 insertions(+), 148 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index 8ff4d13a6a4..c3846db3728 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -1,7 +1,7 @@ 'use strict' const { storage } = require('../../../../../datadog-core') -const { getFirstNonDDPathAndLine } = require('../path-line') +const { getNonDDPathAndLineFromCallsites } = require('../path-line') const { addVulnerability, getVulnerabilityCallSiteList } = require('../vulnerability-reporter') const { getIastContext, getIastStackTraceId } = require('../iast-context') const overheadController = require('../overhead-controller') @@ -29,14 +29,19 @@ class Analyzer extends SinkIastPlugin { _reportEvidence (value, context, evidence) { const callSiteList = getVulnerabilityCallSiteList() - const location = this._getLocation(value, callSiteList) + const nonDDCallSiteList = getNonDDPathAndLineFromCallsites(callSiteList, this._getExcludedPaths()) + + const location = this._getLocation(value, nonDDCallSiteList) if (!this._isExcluded(location)) { - const locationSourceMap = this._replaceLocationFromSourceMap(location) + const originalCallSiteList = nonDDCallSiteList.map(callSite => this._replaceCallsiteFromSourceMap(callSite)) + + const originalLocation = this._getOriginalLocation(originalCallSiteList) const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId() const stackId = getIastStackTraceId(context) - const vulnerability = this._createVulnerability(this._type, evidence, spanId, locationSourceMap, stackId) - addVulnerability(context, vulnerability, callSiteList, stackId) + const vulnerability = this._createVulnerability(this._type, evidence, spanId, originalLocation, stackId) + + addVulnerability(context, vulnerability, originalCallSiteList, stackId) } } @@ -53,23 +58,41 @@ class Analyzer extends SinkIastPlugin { } _getLocation (value, callSiteList) { - return getFirstNonDDPathAndLine(callSiteList, this._getExcludedPaths()) + return callSiteList[0] } - _replaceLocationFromSourceMap (location) { - if (location) { - const { path, line, column } = getOriginalPathAndLineFromSourceMap(location) + _getOriginalLocation (originalCallSiteList) { + const [location] = originalCallSiteList + const originalLocation = {} + + if (location.path) { + originalLocation.path = location.path + } + if (location.line) { + originalLocation.line = location.line + } + if (location.column) { + originalLocation.column = location.column + } + + return originalLocation + } + + _replaceCallsiteFromSourceMap (callsite) { + if (callsite) { + const { path, line, column } = getOriginalPathAndLineFromSourceMap(callsite) if (path) { - location.path = path + callsite.path = path } if (line) { - location.line = line + callsite.line = line } if (column) { - location.column = column + callsite.column = column } } - return location + + return callsite } _getExcludedPaths () {} diff --git a/packages/dd-trace/src/appsec/iast/path-line.js b/packages/dd-trace/src/appsec/iast/path-line.js index 3c034aad8f2..ce93967471a 100644 --- a/packages/dd-trace/src/appsec/iast/path-line.js +++ b/packages/dd-trace/src/appsec/iast/path-line.js @@ -4,10 +4,9 @@ const path = require('path') const process = require('process') const { calculateDDBasePath } = require('../../util') const pathLine = { - getFirstNonDDPathAndLine, getNodeModulesPaths, getRelativePath, - getFirstNonDDPathAndLineFromCallsites, // Exported only for test purposes + getNonDDPathAndLineFromCallsites, calculateDDBasePath, // Exported only for test purposes ddBasePath: calculateDDBasePath(__dirname) // Only for test purposes } @@ -24,22 +23,26 @@ const EXCLUDED_PATH_PREFIXES = [ 'async_hooks' ] -function getFirstNonDDPathAndLineFromCallsites (callsites, externallyExcludedPaths) { - if (callsites) { - for (let i = 0; i < callsites.length; i++) { - const callsite = callsites[i] - const filepath = callsite.getFileName() - if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) { - return { - path: getRelativePath(filepath), - line: callsite.getLineNumber(), - column: callsite.getColumnNumber(), - isInternal: !path.isAbsolute(filepath) - } - } +function getNonDDPathAndLineFromCallsites (callsites, externallyExcludedPaths) { + if (!callsites) { + return [] + } + + const result = [] + + for (const callsite of callsites) { + const filepath = callsite.getFileName() + if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) { + callsite.column = callsite.getLineNumber() + callsite.line = callsite.getColumnNumber() + callsite.path = getRelativePath(filepath) + callsite.isInternal = !path.isAbsolute(filepath) + + result.push(callsite) } } - return null + + return result } function getRelativePath (filepath) { @@ -72,10 +75,6 @@ function isExcluded (callsite, externallyExcludedPaths) { return false } -function getFirstNonDDPathAndLine (callSiteList, externallyExcludedPaths) { - return getFirstNonDDPathAndLineFromCallsites(callSiteList, externallyExcludedPaths) -} - function getNodeModulesPaths (...paths) { const nodeModulesPaths = [] diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index 709783d2985..fcdb1ef33bf 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -50,11 +50,12 @@ function getFramesForMetaStruct (callSiteList, maxDepth = 32) { const callSite = filteredFrames[index] indexedFrames.push({ id: index, - file: callSite.getFileName(), - line: callSite.getLineNumber(), - column: callSite.getColumnNumber(), + file: callSite.file || callSite.getFileName(), + line: callSite.line || callSite.getLineNumber(), + column: callSite.column || callSite.getColumnNumber(), function: callSite.getFunctionName(), - class_name: callSite.getTypeName() + class_name: callSite.getTypeName(), + isNative: callSite.isNative() }) } diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index 3a4ad3a9af6..7e5e6837b5c 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -25,7 +25,7 @@ describe('vulnerability-analyzer', () => { getVulnerabilityCallSiteList: sinon.stub().returns([]) } pathLine = { - getFirstNonDDPathAndLine: sinon.stub().returns(VULNERABILITY_LOCATION) + getNonDDPathAndLineFromCallsites: sinon.stub().returns([VULNERABILITY_LOCATION]) } overheadController = { hasQuota: sinon.stub() @@ -132,7 +132,7 @@ describe('vulnerability-analyzer', () => { }, hash: 5975567724 }, - [], + sinon.match.array, 1 ) }) @@ -289,7 +289,7 @@ describe('vulnerability-analyzer', () => { ANALYZER_TYPE, { value: 'test' }, SPAN_ID, - VULNERABILITY_LOCATION + VULNERABILITY_LOCATION_FROM_SOURCEMAP ) }) }) diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js index aa65c0ff30d..5c6fc30de9c 100644 --- a/packages/dd-trace/test/appsec/iast/path-line.spec.js +++ b/packages/dd-trace/test/appsec/iast/path-line.spec.js @@ -4,25 +4,13 @@ const os = require('os') const { expect } = require('chai') class CallSiteMock { - constructor (fileName, lineNumber, columnNumber = 0) { - this.fileName = fileName - this.lineNumber = lineNumber - this.columnNumber = columnNumber + constructor (file, line, column = 0) { + this.file = file + this.line = line + this.column = column } - getLineNumber () { - return this.lineNumber - } - - getColumnNumber () { - return this.columnNumber - } - - getFileName () { - return this.fileName - } - - isNative () { + get isNative () { return false } } @@ -50,17 +38,6 @@ describe('path-line', function () { }) }) - describe('getFirstNonDDPathAndLine', () => { - it('call does not fail', () => { - const PROJECT_PATH = path.join(rootPath, 'project-path') - const DD_BASE_PATH = path.join(PROJECT_PATH, 'node_modules', 'dd-trace') - const callSiteList = [] - callSiteList.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) - const obj = pathLine.getFirstNonDDPathAndLine(callSiteList, false) - expect(obj).to.not.be.null - }) - }) - describe('calculateDDBasePath', () => { it('/node_modules/dd-trace', () => { const basePath = path.join(rootPath, 'node_modules', 'dd-trace', 'packages', path.sep) @@ -82,18 +59,21 @@ describe('path-line', function () { }) }) - describe('getFirstNonDDPathAndLineFromCallsites', () => { + describe('getNonDDCallsitesFrames', () => { describe('does not fail', () => { it('with null parameter', () => { - pathLine.getFirstNonDDPathAndLineFromCallsites(null) + const result = pathLine.getNonDDCallsitesFrames(null) + expect(result).to.be.an('array').that.is.empty }) it('with empty list parameter', () => { - pathLine.getFirstNonDDPathAndLineFromCallsites([]) + const result = pathLine.getNonDDCallsitesFrames([]) + expect(result).to.be.an('array').that.is.empty }) it('without parameter', () => { - pathLine.getFirstNonDDPathAndLineFromCallsites() + const result = pathLine.getNonDDCallsitesFrames() + expect(result).to.be.an('array').that.is.empty }) }) @@ -114,52 +94,65 @@ describe('path-line', function () { pathLine.ddBasePath = prevDDBasePath }) - it('should return first non DD library when two stack are in dd-trace files and the next is the client line', - () => { - const callsites = [] - const expectedFirstFileOutOfDD = path.join('first', 'file', 'out', 'of', 'dd.js') - const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFirstFileOutOfDD) - const firstFileOutOfDDLineNumber = 13 + it('should return all no DD entries when multiple stack frames are present', () => { + const callsites = [] + const expectedFilePaths = [ + path.join('first', 'file', 'out', 'of', 'dd1.js'), + path.join('second', 'file', 'out', 'of', 'dd2.js') + ] + const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFilePaths[0]) + const secondFileOutOfDD = path.join(PROJECT_PATH, expectedFilePaths[1]) - callsites.push(new CallSiteMock(PATH_AND_LINE_PATH, PATH_AND_LINE_LINE)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 5)) - callsites.push(new CallSiteMock(firstFileOutOfDD, firstFileOutOfDDLineNumber, 42)) - const pathAndLine = pathLine.getFirstNonDDPathAndLineFromCallsites(callsites) - expect(pathAndLine.path).to.be.equals(expectedFirstFileOutOfDD) - expect(pathAndLine.line).to.be.equals(firstFileOutOfDDLineNumber) - expect(pathAndLine.column).to.be.equals(42) - }) + callsites.push(new CallSiteMock(PATH_AND_LINE_PATH, PATH_AND_LINE_LINE)) + callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) + callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) + callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) + + const results = pathLine.getNonDDCallsitesFrames(callsites) + + expect(results).to.have.lengthOf(2) + + expect(results[0].path).to.be.equals(expectedFilePaths[0]) + expect(results[0].line).to.be.equals(13) + expect(results[0].column).to.be.equals(42) - it('should return null when all stack is in dd trace', () => { + expect(results[1].path).to.be.equals(expectedFilePaths[1]) + expect(results[1].line).to.be.equals(20) + expect(results[1].column).to.be.equals(15) + }) + + it('should return an empty array when all stack frames are in dd trace', () => { const callsites = [] callsites.push(new CallSiteMock(PATH_AND_LINE_PATH, PATH_AND_LINE_LINE)) callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 5)) - const pathAndLine = pathLine.getFirstNonDDPathAndLineFromCallsites(callsites) - expect(pathAndLine).to.be.null + callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'another', 'file', 'in', 'dd.js'), 5)) + + const results = pathLine.getNonDDCallsitesFrames(callsites) + expect(results).to.be.an('array').that.is.empty }) DIAGNOSTICS_CHANNEL_PATHS.forEach((dcPath) => { - it(`should not return ${dcPath} path`, () => { + it(`should exclude ${dcPath} from the results`, () => { const callsites = [] - const expectedFirstFileOutOfDD = path.join('first', 'file', 'out', 'of', 'dd.js') - const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFirstFileOutOfDD) - const firstFileOutOfDDLineNumber = 13 + const expectedFilePath = path.join('first', 'file', 'out', 'of', 'dd.js') + const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFilePath) + callsites.push(new CallSiteMock(PATH_AND_LINE_PATH, PATH_AND_LINE_LINE)) callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) callsites.push(new CallSiteMock(dcPath, 25)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 5)) - callsites.push(new CallSiteMock(firstFileOutOfDD, firstFileOutOfDDLineNumber, 42)) - const pathAndLine = pathLine.getFirstNonDDPathAndLineFromCallsites(callsites) - expect(pathAndLine.path).to.be.equals(expectedFirstFileOutOfDD) - expect(pathAndLine.line).to.be.equals(firstFileOutOfDDLineNumber) - expect(pathAndLine.column).to.be.equals(42) + callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) + + const results = pathLine.getNonDDCallsitesFrames(callsites) + expect(results).to.have.lengthOf(1) + + expect(results[0].path).to.be.equals(expectedFilePath) + expect(results[0].line).to.be.equals(13) + expect(results[0].column).to.be.equals(42) }) }) }) - describe('dd-trace is in other directory', () => { + describe('dd-trace is in another directory', () => { const PROJECT_PATH = path.join(tmpdir, 'project-path') const DD_BASE_PATH = path.join(tmpdir, 'dd-tracer-path') const PATH_AND_LINE_PATH = path.join(DD_BASE_PATH, 'packages', @@ -177,37 +170,30 @@ describe('path-line', function () { pathLine.ddBasePath = previousDDBasePath }) - it('two in dd-trace files and the next is the client line', () => { + it('should return all non-DD entries', () => { const callsites = [] - const expectedFilePath = path.join('first', 'file', 'out', 'of', 'dd.js') - const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFilePath) - const firstFileOutOfDDLineNumber = 13 + const expectedFilePaths = [ + path.join('first', 'file', 'out', 'of', 'dd.js'), + path.join('second', 'file', 'out', 'of', 'dd.js') + ] + const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFilePaths[0]) + const secondFileOutOfDD = path.join(PROJECT_PATH, expectedFilePaths[1]) + callsites.push(new CallSiteMock(PATH_AND_LINE_PATH, PATH_AND_LINE_LINE)) callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 5)) - callsites.push(new CallSiteMock(firstFileOutOfDD, firstFileOutOfDDLineNumber, 42)) - const pathAndLine = pathLine.getFirstNonDDPathAndLineFromCallsites(callsites) - expect(pathAndLine.path).to.be.equals(expectedFilePath) - expect(pathAndLine.line).to.be.equals(firstFileOutOfDDLineNumber) - expect(pathAndLine.column).to.be.equals(42) - }) + callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) + callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - DIAGNOSTICS_CHANNEL_PATHS.forEach((dcPath) => { - it(`should not return ${dcPath} path`, () => { - const callsites = [] - const expectedFilePath = path.join('first', 'file', 'out', 'of', 'dd.js') - const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFilePath) - const firstFileOutOfDDLineNumber = 13 - callsites.push(new CallSiteMock(PATH_AND_LINE_PATH, PATH_AND_LINE_LINE)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) - callsites.push(new CallSiteMock(dcPath, 25)) - callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 5)) - callsites.push(new CallSiteMock(firstFileOutOfDD, firstFileOutOfDDLineNumber, 42)) - const pathAndLine = pathLine.getFirstNonDDPathAndLineFromCallsites(callsites) - expect(pathAndLine.path).to.be.equals(expectedFilePath) - expect(pathAndLine.line).to.be.equals(firstFileOutOfDDLineNumber) - expect(pathAndLine.column).to.be.equals(42) - }) + const results = pathLine.getNonDDCallsitesFrames(callsites) + expect(results).to.have.lengthOf(2) + + expect(results[0].path).to.be.equals(expectedFilePaths[0]) + expect(results[0].line).to.be.equals(13) + expect(results[0].column).to.be.equals(42) + + expect(results[1].path).to.be.equals(expectedFilePaths[1]) + expect(results[1].line).to.be.equals(20) + expect(results[1].column).to.be.equals(15) }) }) }) @@ -225,7 +211,13 @@ describe('path-line', function () { e.stack Error.prepareStackTrace = previousPrepareStackTrace Error.stackTraceLimit = previousStackTraceLimit - return callsiteList + return callsiteList.map(callsite => ({ + ...callsite, + file: callsite.getFileName(), + ine: callsite.getLineNumber(), + column: callsite.getColumnNumber(), + isNative: callsite.isNative() + })) } it('should handle windows paths correctly', () => { @@ -233,7 +225,7 @@ describe('path-line', function () { pathLine.ddBasePath = path.join('test', 'base', 'path') const list = getCallSiteInfo() - const firstNonDDPath = pathLine.getFirstNonDDPathAndLineFromCallsites(list) + const firstNonDDPath = pathLine.getNonDDCallsitesFrames(list)[0] const nodeModulesPaths = pathLine.getNodeModulesPaths(__filename) expect(nodeModulesPaths[0]).to.eq(path.join('node_modules', process.cwd(), firstNonDDPath.path)) diff --git a/packages/dd-trace/test/appsec/stack_trace.spec.js b/packages/dd-trace/test/appsec/stack_trace.spec.js index c66e4643ba3..69e0dd895b6 100644 --- a/packages/dd-trace/test/appsec/stack_trace.spec.js +++ b/packages/dd-trace/test/appsec/stack_trace.spec.js @@ -15,7 +15,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => i, getColumnNumber: () => i, getFunctionName: () => `libraryFunction${i}`, - getTypeName: () => `LibraryClass${i}` + getTypeName: () => `LibraryClass${i}`, + isNative: () => false } )).concat( Array(10).fill().map((_, i) => ( @@ -24,7 +25,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => i, getColumnNumber: () => i, getFunctionName: () => `function${i}`, - getTypeName: () => `Class${i}` + getTypeName: () => `Class${i}`, + isNative: () => false } )) ).concat([ @@ -33,7 +35,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => null, getColumnNumber: () => null, getFunctionName: () => null, - getTypeName: () => null + getTypeName: () => null, + isNative: () => false } ]) @@ -44,7 +47,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `Class${i}` + class_name: `Class${i}`, + isNative: false } )) .concat([ @@ -54,7 +58,8 @@ describe('Stack trace reporter', () => { line: null, column: null, function: null, - class_name: null + class_name: null, + isNative: false } ]) @@ -75,7 +80,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => i, getColumnNumber: () => i, getFunctionName: () => `function${i}`, - getTypeName: () => `type${i}` + getTypeName: () => `type${i}`, + isNative: () => false } )) @@ -101,7 +107,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) @@ -127,7 +134,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) @@ -157,7 +165,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) @@ -245,7 +254,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => i, getColumnNumber: () => i, getFunctionName: () => `function${i}`, - getTypeName: () => `type${i}` + getTypeName: () => `type${i}`, + isNative: () => false } )) @@ -260,7 +270,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) @@ -279,7 +290,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => 314, getColumnNumber: () => 271, getFunctionName: () => 'libraryFunction', - getTypeName: () => 'libraryType' + getTypeName: () => 'libraryType', + isNative: () => false } ].concat(Array(120).fill().map((_, i) => ( { @@ -287,7 +299,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => i, getColumnNumber: () => i, getFunctionName: () => `function${i}`, - getTypeName: () => `type${i}` + getTypeName: () => `type${i}`, + isNative: () => false } )).concat([ { @@ -295,7 +308,8 @@ describe('Stack trace reporter', () => { getLineNumber: () => 271, getColumnNumber: () => 314, getFunctionName: () => 'libraryFunction', - getTypeName: () => 'libraryType' + getTypeName: () => 'libraryType', + isNative: () => false } ])) const expectedFrames = [0, 1, 2, 118, 119].map(i => ( @@ -305,7 +319,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) @@ -325,7 +340,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) @@ -345,7 +361,8 @@ describe('Stack trace reporter', () => { line: i, column: i, function: `function${i}`, - class_name: `type${i}` + class_name: `type${i}`, + isNative: false } )) From e03bfb0cf027677dcb8eeb79cffe67b75698eea5 Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 13 Jan 2025 16:32:13 +0100 Subject: [PATCH 05/14] fix path-line tests --- .../dd-trace/src/appsec/iast/path-line.js | 4 +- .../test/appsec/iast/path-line.spec.js | 49 +++++++++++-------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/path-line.js b/packages/dd-trace/src/appsec/iast/path-line.js index ce93967471a..34c1df6880d 100644 --- a/packages/dd-trace/src/appsec/iast/path-line.js +++ b/packages/dd-trace/src/appsec/iast/path-line.js @@ -33,8 +33,8 @@ function getNonDDPathAndLineFromCallsites (callsites, externallyExcludedPaths) { for (const callsite of callsites) { const filepath = callsite.getFileName() if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) { - callsite.column = callsite.getLineNumber() - callsite.line = callsite.getColumnNumber() + callsite.column = callsite.getColumnNumber() + callsite.line = callsite.getLineNumber() callsite.path = getRelativePath(filepath) callsite.isInternal = !path.isAbsolute(filepath) diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js index 5c6fc30de9c..a6bb77efd13 100644 --- a/packages/dd-trace/test/appsec/iast/path-line.spec.js +++ b/packages/dd-trace/test/appsec/iast/path-line.spec.js @@ -4,13 +4,25 @@ const os = require('os') const { expect } = require('chai') class CallSiteMock { - constructor (file, line, column = 0) { - this.file = file - this.line = line - this.column = column + constructor (fileName, lineNumber, columnNumber = 0) { + this.fileName = fileName + this.lineNumber = lineNumber + this.columnNumber = columnNumber } - get isNative () { + getLineNumber () { + return this.lineNumber + } + + getColumnNumber () { + return this.columnNumber + } + + getFileName () { + return this.fileName + } + + isNative () { return false } } @@ -59,20 +71,20 @@ describe('path-line', function () { }) }) - describe('getNonDDCallsitesFrames', () => { + describe('getNonDDPathAndLineFromCallsites', () => { describe('does not fail', () => { it('with null parameter', () => { - const result = pathLine.getNonDDCallsitesFrames(null) + const result = pathLine.getNonDDPathAndLineFromCallsites(null) expect(result).to.be.an('array').that.is.empty }) it('with empty list parameter', () => { - const result = pathLine.getNonDDCallsitesFrames([]) + const result = pathLine.getNonDDPathAndLineFromCallsites([]) expect(result).to.be.an('array').that.is.empty }) it('without parameter', () => { - const result = pathLine.getNonDDCallsitesFrames() + const result = pathLine.getNonDDPathAndLineFromCallsites() expect(result).to.be.an('array').that.is.empty }) }) @@ -108,7 +120,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - const results = pathLine.getNonDDCallsitesFrames(callsites) + const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) expect(results).to.have.lengthOf(2) @@ -127,7 +139,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'another', 'file', 'in', 'dd.js'), 5)) - const results = pathLine.getNonDDCallsitesFrames(callsites) + const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) expect(results).to.be.an('array').that.is.empty }) @@ -142,7 +154,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(dcPath, 25)) callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) - const results = pathLine.getNonDDCallsitesFrames(callsites) + const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) expect(results).to.have.lengthOf(1) expect(results[0].path).to.be.equals(expectedFilePath) @@ -184,7 +196,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - const results = pathLine.getNonDDCallsitesFrames(callsites) + const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) expect(results).to.have.lengthOf(2) expect(results[0].path).to.be.equals(expectedFilePaths[0]) @@ -211,13 +223,8 @@ describe('path-line', function () { e.stack Error.prepareStackTrace = previousPrepareStackTrace Error.stackTraceLimit = previousStackTraceLimit - return callsiteList.map(callsite => ({ - ...callsite, - file: callsite.getFileName(), - ine: callsite.getLineNumber(), - column: callsite.getColumnNumber(), - isNative: callsite.isNative() - })) + + return callsiteList } it('should handle windows paths correctly', () => { @@ -225,7 +232,7 @@ describe('path-line', function () { pathLine.ddBasePath = path.join('test', 'base', 'path') const list = getCallSiteInfo() - const firstNonDDPath = pathLine.getNonDDCallsitesFrames(list)[0] + const firstNonDDPath = pathLine.getNonDDPathAndLineFromCallsites(list)[0] const nodeModulesPaths = pathLine.getNodeModulesPaths(__filename) expect(nodeModulesPaths[0]).to.eq(path.join('node_modules', process.cwd(), firstNonDDPath.path)) From a4f7df6ed44a845b61f68ce7dd3e2965e39aac75 Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 14 Jan 2025 11:17:10 +0100 Subject: [PATCH 06/14] use frames instead of call list --- .../iast/analyzers/vulnerability-analyzer.js | 19 ++-- .../dd-trace/src/appsec/iast/path-line.js | 16 ++-- .../src/appsec/iast/vulnerability-reporter.js | 13 ++- packages/dd-trace/src/appsec/rasp/utils.js | 7 +- packages/dd-trace/src/appsec/stack_trace.js | 22 ++--- .../analyzers/vulnerability-analyzer.spec.js | 5 +- .../test/appsec/iast/path-line.spec.js | 94 +++++++++---------- .../dd-trace/test/appsec/rasp/utils.spec.js | 2 +- .../dd-trace/test/appsec/stack_trace.spec.js | 48 +++++++--- 9 files changed, 116 insertions(+), 110 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index c3846db3728..1c123bd579e 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -1,8 +1,8 @@ 'use strict' const { storage } = require('../../../../../datadog-core') -const { getNonDDPathAndLineFromCallsites } = require('../path-line') -const { addVulnerability, getVulnerabilityCallSiteList } = require('../vulnerability-reporter') +const { getNonDDFrames } = require('../path-line') +const { addVulnerability, getVulnerabilityCallSiteFrames } = require('../vulnerability-reporter') const { getIastContext, getIastStackTraceId } = require('../iast-context') const overheadController = require('../overhead-controller') const { SinkIastPlugin } = require('../iast-plugin') @@ -28,13 +28,13 @@ class Analyzer extends SinkIastPlugin { } _reportEvidence (value, context, evidence) { - const callSiteList = getVulnerabilityCallSiteList() - const nonDDCallSiteList = getNonDDPathAndLineFromCallsites(callSiteList, this._getExcludedPaths()) + const callSiteFrames = getVulnerabilityCallSiteFrames() + const nonDDCallSiteFrames = getNonDDFrames(callSiteFrames, this._getExcludedPaths()) - const location = this._getLocation(value, nonDDCallSiteList) + const location = this._getLocation(value, nonDDCallSiteFrames) if (!this._isExcluded(location)) { - const originalCallSiteList = nonDDCallSiteList.map(callSite => this._replaceCallsiteFromSourceMap(callSite)) + const originalCallSiteList = nonDDCallSiteFrames.map(callSite => this._replaceCallsiteFromSourceMap(callSite)) const originalLocation = this._getOriginalLocation(originalCallSiteList) const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId() @@ -65,13 +65,13 @@ class Analyzer extends SinkIastPlugin { const [location] = originalCallSiteList const originalLocation = {} - if (location.path) { + if (location?.path) { originalLocation.path = location.path } - if (location.line) { + if (location?.line) { originalLocation.line = location.line } - if (location.column) { + if (location?.column) { originalLocation.column = location.column } @@ -82,6 +82,7 @@ class Analyzer extends SinkIastPlugin { if (callsite) { const { path, line, column } = getOriginalPathAndLineFromSourceMap(callsite) if (path) { + callsite.file = path callsite.path = path } if (line) { diff --git a/packages/dd-trace/src/appsec/iast/path-line.js b/packages/dd-trace/src/appsec/iast/path-line.js index 34c1df6880d..e6e6bb00e55 100644 --- a/packages/dd-trace/src/appsec/iast/path-line.js +++ b/packages/dd-trace/src/appsec/iast/path-line.js @@ -6,7 +6,7 @@ const { calculateDDBasePath } = require('../../util') const pathLine = { getNodeModulesPaths, getRelativePath, - getNonDDPathAndLineFromCallsites, + getNonDDFrames, calculateDDBasePath, // Exported only for test purposes ddBasePath: calculateDDBasePath(__dirname) // Only for test purposes } @@ -23,18 +23,16 @@ const EXCLUDED_PATH_PREFIXES = [ 'async_hooks' ] -function getNonDDPathAndLineFromCallsites (callsites, externallyExcludedPaths) { - if (!callsites) { +function getNonDDFrames (callSiteFrames, externallyExcludedPaths) { + if (!callSiteFrames) { return [] } const result = [] - for (const callsite of callsites) { - const filepath = callsite.getFileName() + for (const callsite of callSiteFrames) { + const filepath = callsite.file if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) { - callsite.column = callsite.getColumnNumber() - callsite.line = callsite.getLineNumber() callsite.path = getRelativePath(filepath) callsite.isInternal = !path.isAbsolute(filepath) @@ -50,8 +48,8 @@ function getRelativePath (filepath) { } function isExcluded (callsite, externallyExcludedPaths) { - if (callsite.isNative()) return true - const filename = callsite.getFileName() + if (callsite.isNative) return true + const filename = callsite.file if (!filename) { return true } diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 264e7f916e6..282fa3b57d6 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -6,7 +6,7 @@ const { IAST_ENABLED_TAG_KEY, IAST_JSON_TAG_KEY } = require('./tags') const standalone = require('../standalone') const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') const { keepTrace } = require('../../priority_sampler') -const { reportStackTrace, getCallSiteList, STACK_TRACE_NAMESPACES } = require('../stack_trace') +const { reportStackTrace, getFramesForMetaStruct, STACK_TRACE_NAMESPACES } = require('../stack_trace') const VULNERABILITIES_KEY = 'vulnerabilities' const VULNERABILITY_HASHES_MAX_SIZE = 1000 @@ -19,7 +19,7 @@ let deduplicationEnabled = true let stackTraceMaxDepth let maxStackTraces -function addVulnerability (iastContext, vulnerability, callSiteList, stackId) { +function addVulnerability (iastContext, vulnerability, callSiteFrames, stackId) { if (vulnerability?.evidence && vulnerability?.type && vulnerability?.location) { if (deduplicationEnabled && isDuplicatedVulnerability(vulnerability)) return @@ -47,9 +47,8 @@ function addVulnerability (iastContext, vulnerability, callSiteList, stackId) { reportStackTrace( iastContext?.rootSpan, stackId, - stackTraceMaxDepth, maxStackTraces, - callSiteList, + callSiteFrames, STACK_TRACE_NAMESPACES.IAST ) @@ -106,8 +105,8 @@ function isDuplicatedVulnerability (vulnerability) { return VULNERABILITY_HASHES.get(`${vulnerability.type}${vulnerability.hash}`) } -function getVulnerabilityCallSiteList () { - return getCallSiteList(stackTraceMaxDepth) +function getVulnerabilityCallSiteFrames () { + return getFramesForMetaStruct(stackTraceMaxDepth) } function start (config, _tracer) { @@ -133,7 +132,7 @@ function stop () { module.exports = { addVulnerability, sendVulnerabilities, - getVulnerabilityCallSiteList, + getVulnerabilityCallSiteFrames, clearCache, start, stop diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index 4908d8c8687..439ceb7be91 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -1,7 +1,7 @@ 'use strict' const web = require('../../plugins/util/web') -const { getCallSiteList, reportStackTrace } = require('../stack_trace') +const { getFramesForMetaStruct, reportStackTrace } = require('../stack_trace') const { getBlockingAction } = require('../blocking') const log = require('../../log') @@ -34,15 +34,14 @@ function handleResult (actions, req, res, abortController, config) { const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace if (generateStackTraceAction && enabled) { - const callSiteList = getCallSiteList(maxDepth) + const frames = getFramesForMetaStruct(maxDepth) const rootSpan = web.root(req) reportStackTrace( rootSpan, generateStackTraceAction.stack_id, - maxDepth, maxStackTraces, - callSiteList + frames ) } diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index fcdb1ef33bf..d9fed678930 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -12,8 +12,6 @@ const STACK_TRACE_NAMESPACES = { } function getCallSiteList (maxDepth = 100) { - if (maxDepth < 1) maxDepth = Infinity - const previousPrepareStackTrace = Error.prepareStackTrace const previousStackTraceLimit = Error.stackTraceLimit let callsiteList @@ -39,7 +37,10 @@ function filterOutFramesFromLibrary (callSiteList) { return callSiteList.filter(callSite => !callSite.getFileName()?.startsWith(ddBasePath)) } -function getFramesForMetaStruct (callSiteList, maxDepth = 32) { +function getFramesForMetaStruct (maxDepth = 32, callSiteListGetter = getCallSiteList) { + if (maxDepth < 1) maxDepth = Infinity + + const callSiteList = callSiteListGetter(maxDepth) const filteredFrames = filterOutFramesFromLibrary(callSiteList) const half = filteredFrames.length > maxDepth ? Math.round(maxDepth / 2) : Infinity @@ -50,9 +51,9 @@ function getFramesForMetaStruct (callSiteList, maxDepth = 32) { const callSite = filteredFrames[index] indexedFrames.push({ id: index, - file: callSite.file || callSite.getFileName(), - line: callSite.line || callSite.getLineNumber(), - column: callSite.column || callSite.getColumnNumber(), + file: callSite.getFileName(), + line: callSite.getLineNumber(), + column: callSite.getColumnNumber(), function: callSite.getFunctionName(), class_name: callSite.getTypeName(), isNative: callSite.isNative() @@ -63,12 +64,11 @@ function getFramesForMetaStruct (callSiteList, maxDepth = 32) { } function reportStackTrace ( - rootSpan, stackId, maxDepth, maxStackTraces, callSiteList, namespace = STACK_TRACE_NAMESPACES.RASP) { + rootSpan, stackId, maxStackTraces, frames, namespace = STACK_TRACE_NAMESPACES.RASP) { if (!rootSpan) return if (maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.[namespace]?.length ?? 0) < maxStackTraces) { - if (maxDepth < 1) maxDepth = Infinity - if (!Array.isArray(callSiteList)) return + if (!Array.isArray(frames)) return if (!rootSpan.meta_struct) { rootSpan.meta_struct = {} @@ -82,8 +82,6 @@ function reportStackTrace ( rootSpan.meta_struct['_dd.stack'][namespace] = [] } - const frames = getFramesForMetaStruct(callSiteList, maxDepth) - rootSpan.meta_struct['_dd.stack'][namespace].push({ id: stackId, language: 'nodejs', @@ -93,7 +91,7 @@ function reportStackTrace ( } module.exports = { - getCallSiteList, + getFramesForMetaStruct, reportStackTrace, STACK_TRACE_NAMESPACES } diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index 7e5e6837b5c..c43c37b9da6 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -7,7 +7,7 @@ describe('vulnerability-analyzer', () => { const VULNERABLE_VALUE = 'VULNERABLE_VALUE' const VULNERABILITY = 'VULNERABILITY' const VULNERABILITY_LOCATION = { path: 'VULNERABILITY_LOCATION', line: 11 } - const VULNERABILITY_LOCATION_FROM_SOURCEMAP = { path: 'VULNERABILITY_LOCATION_FROM_SOURCEMAP', line: 42 } + const VULNERABILITY_LOCATION_FROM_SOURCEMAP = { path: 'VULNERABILITY_LOCATION_FROM_SOURCEMAP', line: 42, column: 21 } const ANALYZER_TYPE = 'TEST_ANALYZER' const SPAN_ID = '123456' @@ -127,8 +127,7 @@ describe('vulnerability-analyzer', () => { }, location: { spanId: '123456', - path: 'VULNERABILITY_LOCATION_FROM_SOURCEMAP', - line: 42 + ...VULNERABILITY_LOCATION_FROM_SOURCEMAP }, hash: 5975567724 }, diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js index a6bb77efd13..bce2be3ab33 100644 --- a/packages/dd-trace/test/appsec/iast/path-line.spec.js +++ b/packages/dd-trace/test/appsec/iast/path-line.spec.js @@ -5,24 +5,12 @@ const { expect } = require('chai') class CallSiteMock { constructor (fileName, lineNumber, columnNumber = 0) { - this.fileName = fileName - this.lineNumber = lineNumber - this.columnNumber = columnNumber + this.file = fileName + this.line = lineNumber + this.column = columnNumber } - getLineNumber () { - return this.lineNumber - } - - getColumnNumber () { - return this.columnNumber - } - - getFileName () { - return this.fileName - } - - isNative () { + get isNative () { return false } } @@ -71,20 +59,20 @@ describe('path-line', function () { }) }) - describe('getNonDDPathAndLineFromCallsites', () => { + describe('getNonDDFrames', () => { describe('does not fail', () => { it('with null parameter', () => { - const result = pathLine.getNonDDPathAndLineFromCallsites(null) + const result = pathLine.getNonDDFrames(null) expect(result).to.be.an('array').that.is.empty }) it('with empty list parameter', () => { - const result = pathLine.getNonDDPathAndLineFromCallsites([]) + const result = pathLine.getNonDDFrames([]) expect(result).to.be.an('array').that.is.empty }) it('without parameter', () => { - const result = pathLine.getNonDDPathAndLineFromCallsites() + const result = pathLine.getNonDDFrames() expect(result).to.be.an('array').that.is.empty }) }) @@ -120,7 +108,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) + const results = pathLine.getNonDDFrames(callsites) expect(results).to.have.lengthOf(2) @@ -139,7 +127,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'another', 'file', 'in', 'dd.js'), 5)) - const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) + const results = pathLine.getNonDDFrames(callsites) expect(results).to.be.an('array').that.is.empty }) @@ -154,7 +142,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(dcPath, 25)) callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) - const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) + const results = pathLine.getNonDDFrames(callsites) expect(results).to.have.lengthOf(1) expect(results[0].path).to.be.equals(expectedFilePath) @@ -196,7 +184,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - const results = pathLine.getNonDDPathAndLineFromCallsites(callsites) + const results = pathLine.getNonDDFrames(callsites) expect(results).to.have.lengthOf(2) expect(results[0].path).to.be.equals(expectedFilePaths[0]) @@ -211,34 +199,36 @@ describe('path-line', function () { }) describe('getNodeModulesPaths', () => { - function getCallSiteInfo () { - const previousPrepareStackTrace = Error.prepareStackTrace - const previousStackTraceLimit = Error.stackTraceLimit - let callsiteList - Error.stackTraceLimit = 100 - Error.prepareStackTrace = function (_, callsites) { - callsiteList = callsites - } - const e = new Error() - e.stack - Error.prepareStackTrace = previousPrepareStackTrace - Error.stackTraceLimit = previousStackTraceLimit - - return callsiteList - } - - it('should handle windows paths correctly', () => { - const basePath = pathLine.ddBasePath - pathLine.ddBasePath = path.join('test', 'base', 'path') - - const list = getCallSiteInfo() - const firstNonDDPath = pathLine.getNonDDPathAndLineFromCallsites(list)[0] - - const nodeModulesPaths = pathLine.getNodeModulesPaths(__filename) - expect(nodeModulesPaths[0]).to.eq(path.join('node_modules', process.cwd(), firstNonDDPath.path)) - - pathLine.ddBasePath = basePath - }) + // function getCallSiteInfo () { + // const previousPrepareStackTrace = Error.prepareStackTrace + // const previousStackTraceLimit = Error.stackTraceLimit + // let callsiteList + // Error.stackTraceLimit = 100 + // Error.prepareStackTrace = function (_, callsites) { + // callsiteList = callsites + // } + // const e = new Error() + // e.stack + // Error.prepareStackTrace = previousPrepareStackTrace + // Error.stackTraceLimit = previousStackTraceLimit + + // return callsiteList + // } + // TODO: propose another test similar to this + // it('should handle windows paths correctly', () => { + // const basePath = pathLine.ddBasePath + // pathLine.ddBasePath = path.join('test', 'base', 'path') + // const { getFramesForMetaStruct } = require('../../../src/appsec/stack_trace') + + // const list = getFramesForMetaStruct(32, getCallSiteInfo) + // const firstNonDDPath = pathLine.getNonDDFrames(list)[0] + + // const nodeModulesPaths = pathLine.getNodeModulesPaths(__filename) + + // expect(nodeModulesPaths[0]).to.eq(path.join('node_modules', process.cwd(), firstNonDDPath.path)) + + // pathLine.ddBasePath = basePath + // }) it('should convert / to \\ in windows platforms', () => { const dirname = __dirname diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js index 2f6918dd0ab..0389fe163d2 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.spec.js +++ b/packages/dd-trace/test/appsec/rasp/utils.spec.js @@ -45,7 +45,7 @@ describe('RASP - utils.js', () => { web.root.returns(rootSpan) utils.handleResult(result, req, undefined, undefined, config) - sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 42, 2, []) + sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 2, sinon.match.array) }) it('should not report stack trace when no action is present in waf result', () => { diff --git a/packages/dd-trace/test/appsec/stack_trace.spec.js b/packages/dd-trace/test/appsec/stack_trace.spec.js index 69e0dd895b6..0808c2b32d3 100644 --- a/packages/dd-trace/test/appsec/stack_trace.spec.js +++ b/packages/dd-trace/test/appsec/stack_trace.spec.js @@ -3,11 +3,11 @@ const { assert } = require('chai') const path = require('path') -const { reportStackTrace } = require('../../src/appsec/stack_trace') +const { reportStackTrace, getFramesForMetaStruct } = require('../../src/appsec/stack_trace') describe('Stack trace reporter', () => { describe('frame filtering', () => { - it('should filer out frames from library', () => { + it('should filter out frames from library', () => { const callSiteList = Array(10).fill().map((_, i) => ( { @@ -67,7 +67,9 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 const maxStackTraces = 2 - reportStackTrace(rootSpan, stackId, maxDepth, maxStackTraces, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, maxStackTraces, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -112,7 +114,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].language, 'nodejs') @@ -139,7 +143,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].language, 'nodejs') @@ -170,7 +176,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[1].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[1].language, 'nodejs') @@ -190,7 +198,9 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 2) assert.property(rootSpan.meta_struct, 'another_tag') @@ -208,7 +218,9 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - reportStackTrace(rootSpan, stackId, maxDepth, 0, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 0, frames) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 3) assert.property(rootSpan.meta_struct, 'another_tag') @@ -226,7 +238,9 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - reportStackTrace(rootSpan, stackId, maxDepth, -1, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, -1, frames) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 3) assert.property(rootSpan.meta_struct, 'another_tag') @@ -275,7 +289,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -324,7 +340,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteListWithLibraryFrames) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteListWithLibraryFrames) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -345,7 +363,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -366,7 +386,9 @@ describe('Stack trace reporter', () => { } )) - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + + reportStackTrace(rootSpan, stackId, 2, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) From a9f199cb9fabc6f74fc9f56836c20355a458b7e5 Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 14 Jan 2025 16:56:41 +0100 Subject: [PATCH 07/14] fix hardcoded-analyzers tests --- .../appsec/iast/analyzers/cookie-analyzer.js | 6 +- .../iast/analyzers/vulnerability-analyzer.js | 36 +++++---- .../dd-trace/src/appsec/iast/path-line.js | 4 +- .../src/appsec/iast/vulnerability-reporter.js | 4 +- packages/dd-trace/src/appsec/rasp/utils.js | 4 +- packages/dd-trace/src/appsec/stack_trace.js | 4 +- .../hardcoded-password-analyzer.spec.js | 2 + .../hardcoded-secret-analyzer.spec.js | 2 + .../test/appsec/iast/path-line.spec.js | 77 ++++++++++--------- .../dd-trace/test/appsec/stack_trace.spec.js | 24 +++--- 10 files changed, 87 insertions(+), 76 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js index 75117d78bde..836908f36e4 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js @@ -54,15 +54,15 @@ class CookieAnalyzer extends Analyzer { return super._checkOCE(context, value) } - _getLocation (value, callSiteList) { + _getLocation (value, callSiteFrames) { if (!value) { - return super._getLocation(value, callSiteList) + return super._getLocation(value, callSiteFrames) } if (value.location) { return value.location } - const location = super._getLocation(value, callSiteList) + const location = super._getLocation(value, callSiteFrames) value.location = location return location } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index 1c123bd579e..d5dcc96ed51 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -1,7 +1,7 @@ 'use strict' const { storage } = require('../../../../../datadog-core') -const { getNonDDFrames } = require('../path-line') +const { getNonDDCallSiteFrames } = require('../path-line') const { addVulnerability, getVulnerabilityCallSiteFrames } = require('../vulnerability-reporter') const { getIastContext, getIastStackTraceId } = require('../iast-context') const overheadController = require('../overhead-controller') @@ -29,17 +29,23 @@ class Analyzer extends SinkIastPlugin { _reportEvidence (value, context, evidence) { const callSiteFrames = getVulnerabilityCallSiteFrames() - const nonDDCallSiteFrames = getNonDDFrames(callSiteFrames, this._getExcludedPaths()) + const nonDDCallSiteFrames = getNonDDCallSiteFrames(callSiteFrames, this._getExcludedPaths()) const location = this._getLocation(value, nonDDCallSiteFrames) if (!this._isExcluded(location)) { - const originalCallSiteList = nonDDCallSiteFrames.map(callSite => this._replaceCallsiteFromSourceMap(callSite)) + const originalCallSiteList = nonDDCallSiteFrames.map(callsite => this._replaceCallsiteFromSourceMap(callsite)) - const originalLocation = this._getOriginalLocation(originalCallSiteList) + const originalLocation = this._getOriginalLocation(location) const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId() const stackId = getIastStackTraceId(context) - const vulnerability = this._createVulnerability(this._type, evidence, spanId, originalLocation, stackId) + const vulnerability = this._createVulnerability( + this._type, + evidence, + spanId, + originalLocation, + stackId + ) addVulnerability(context, vulnerability, originalCallSiteList, stackId) } @@ -57,22 +63,22 @@ class Analyzer extends SinkIastPlugin { return { value } } - _getLocation (value, callSiteList) { - return callSiteList[0] + _getLocation (value, callSiteFrames) { + return callSiteFrames[0] } - _getOriginalLocation (originalCallSiteList) { - const [location] = originalCallSiteList + _getOriginalLocation (location) { + const locationFromSourceMap = this._replaceCallsiteFromSourceMap(location) const originalLocation = {} - if (location?.path) { - originalLocation.path = location.path + if (locationFromSourceMap?.path) { + originalLocation.path = locationFromSourceMap.path } - if (location?.line) { - originalLocation.line = location.line + if (locationFromSourceMap?.line) { + originalLocation.line = locationFromSourceMap.line } - if (location?.column) { - originalLocation.column = location.column + if (locationFromSourceMap?.column) { + originalLocation.column = locationFromSourceMap.column } return originalLocation diff --git a/packages/dd-trace/src/appsec/iast/path-line.js b/packages/dd-trace/src/appsec/iast/path-line.js index e6e6bb00e55..1163bb8d604 100644 --- a/packages/dd-trace/src/appsec/iast/path-line.js +++ b/packages/dd-trace/src/appsec/iast/path-line.js @@ -6,7 +6,7 @@ const { calculateDDBasePath } = require('../../util') const pathLine = { getNodeModulesPaths, getRelativePath, - getNonDDFrames, + getNonDDCallSiteFrames, calculateDDBasePath, // Exported only for test purposes ddBasePath: calculateDDBasePath(__dirname) // Only for test purposes } @@ -23,7 +23,7 @@ const EXCLUDED_PATH_PREFIXES = [ 'async_hooks' ] -function getNonDDFrames (callSiteFrames, externallyExcludedPaths) { +function getNonDDCallSiteFrames (callSiteFrames, externallyExcludedPaths) { if (!callSiteFrames) { return [] } diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 282fa3b57d6..cfdc2ee49cd 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -6,7 +6,7 @@ const { IAST_ENABLED_TAG_KEY, IAST_JSON_TAG_KEY } = require('./tags') const standalone = require('../standalone') const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') const { keepTrace } = require('../../priority_sampler') -const { reportStackTrace, getFramesForMetaStruct, STACK_TRACE_NAMESPACES } = require('../stack_trace') +const { reportStackTrace, getCallsiteFrames, STACK_TRACE_NAMESPACES } = require('../stack_trace') const VULNERABILITIES_KEY = 'vulnerabilities' const VULNERABILITY_HASHES_MAX_SIZE = 1000 @@ -106,7 +106,7 @@ function isDuplicatedVulnerability (vulnerability) { } function getVulnerabilityCallSiteFrames () { - return getFramesForMetaStruct(stackTraceMaxDepth) + return getCallsiteFrames(stackTraceMaxDepth) } function start (config, _tracer) { diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index 439ceb7be91..bf17b9414c2 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -1,7 +1,7 @@ 'use strict' const web = require('../../plugins/util/web') -const { getFramesForMetaStruct, reportStackTrace } = require('../stack_trace') +const { getCallsiteFrames, reportStackTrace } = require('../stack_trace') const { getBlockingAction } = require('../blocking') const log = require('../../log') @@ -34,7 +34,7 @@ function handleResult (actions, req, res, abortController, config) { const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace if (generateStackTraceAction && enabled) { - const frames = getFramesForMetaStruct(maxDepth) + const frames = getCallsiteFrames(maxDepth) const rootSpan = web.root(req) reportStackTrace( diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index d9fed678930..72e97cc8571 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -37,7 +37,7 @@ function filterOutFramesFromLibrary (callSiteList) { return callSiteList.filter(callSite => !callSite.getFileName()?.startsWith(ddBasePath)) } -function getFramesForMetaStruct (maxDepth = 32, callSiteListGetter = getCallSiteList) { +function getCallsiteFrames (maxDepth = 32, callSiteListGetter = getCallSiteList) { if (maxDepth < 1) maxDepth = Infinity const callSiteList = callSiteListGetter(maxDepth) @@ -91,7 +91,7 @@ function reportStackTrace ( } module.exports = { - getFramesForMetaStruct, + getCallsiteFrames, reportStackTrace, STACK_TRACE_NAMESPACES } diff --git a/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-password-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-password-analyzer.spec.js index e20c83ef33d..16fe264328c 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-password-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-password-analyzer.spec.js @@ -10,6 +10,7 @@ const Config = require('../../../../src/config') const hardcodedPasswordAnalyzer = require('../../../../src/appsec/iast/analyzers/hardcoded-password-analyzer') const iast = require('../../../../src/appsec/iast') +const vulnerabilityReporter = require('../../../../src/appsec/iast/vulnerability-reporter') const ruleId = 'hardcoded-password' const samples = [ @@ -131,6 +132,7 @@ describe('Hardcoded Password Analyzer', () => { afterEach(() => { iast.disable() + vulnerabilityReporter.clearCache() }) afterEach(() => { diff --git a/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-secret-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-secret-analyzer.spec.js index 67d00a8b53a..b65aed0a614 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-secret-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/hardcoded-secret-analyzer.spec.js @@ -11,6 +11,7 @@ const { NameAndValue, ValueOnly } = require('../../../../src/appsec/iast/analyze const hardcodedSecretAnalyzer = require('../../../../src/appsec/iast/analyzers/hardcoded-secret-analyzer') const { suite } = require('./resources/hardcoded-secrets-suite.json') const iast = require('../../../../src/appsec/iast') +const vulnerabilityReporter = require('../../../../src/appsec/iast/vulnerability-reporter') describe('Hardcoded Secret Analyzer', () => { describe('unit test', () => { @@ -101,6 +102,7 @@ describe('Hardcoded Secret Analyzer', () => { afterEach(() => { iast.disable() + vulnerabilityReporter.clearCache() }) afterEach(() => { diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js index bce2be3ab33..11555a11e92 100644 --- a/packages/dd-trace/test/appsec/iast/path-line.spec.js +++ b/packages/dd-trace/test/appsec/iast/path-line.spec.js @@ -59,20 +59,20 @@ describe('path-line', function () { }) }) - describe('getNonDDFrames', () => { + describe('getNonDDCallSiteFrames', () => { describe('does not fail', () => { it('with null parameter', () => { - const result = pathLine.getNonDDFrames(null) + const result = pathLine.getNonDDCallSiteFrames(null) expect(result).to.be.an('array').that.is.empty }) it('with empty list parameter', () => { - const result = pathLine.getNonDDFrames([]) + const result = pathLine.getNonDDCallSiteFrames([]) expect(result).to.be.an('array').that.is.empty }) it('without parameter', () => { - const result = pathLine.getNonDDFrames() + const result = pathLine.getNonDDCallSiteFrames() expect(result).to.be.an('array').that.is.empty }) }) @@ -108,7 +108,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - const results = pathLine.getNonDDFrames(callsites) + const results = pathLine.getNonDDCallSiteFrames(callsites) expect(results).to.have.lengthOf(2) @@ -127,7 +127,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'other', 'file', 'in', 'dd.js'), 89)) callsites.push(new CallSiteMock(path.join(DD_BASE_PATH, 'another', 'file', 'in', 'dd.js'), 5)) - const results = pathLine.getNonDDFrames(callsites) + const results = pathLine.getNonDDCallSiteFrames(callsites) expect(results).to.be.an('array').that.is.empty }) @@ -142,7 +142,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(dcPath, 25)) callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) - const results = pathLine.getNonDDFrames(callsites) + const results = pathLine.getNonDDCallSiteFrames(callsites) expect(results).to.have.lengthOf(1) expect(results[0].path).to.be.equals(expectedFilePath) @@ -184,7 +184,7 @@ describe('path-line', function () { callsites.push(new CallSiteMock(firstFileOutOfDD, 13, 42)) callsites.push(new CallSiteMock(secondFileOutOfDD, 20, 15)) - const results = pathLine.getNonDDFrames(callsites) + const results = pathLine.getNonDDCallSiteFrames(callsites) expect(results).to.have.lengthOf(2) expect(results[0].path).to.be.equals(expectedFilePaths[0]) @@ -199,36 +199,37 @@ describe('path-line', function () { }) describe('getNodeModulesPaths', () => { - // function getCallSiteInfo () { - // const previousPrepareStackTrace = Error.prepareStackTrace - // const previousStackTraceLimit = Error.stackTraceLimit - // let callsiteList - // Error.stackTraceLimit = 100 - // Error.prepareStackTrace = function (_, callsites) { - // callsiteList = callsites - // } - // const e = new Error() - // e.stack - // Error.prepareStackTrace = previousPrepareStackTrace - // Error.stackTraceLimit = previousStackTraceLimit - - // return callsiteList - // } - // TODO: propose another test similar to this - // it('should handle windows paths correctly', () => { - // const basePath = pathLine.ddBasePath - // pathLine.ddBasePath = path.join('test', 'base', 'path') - // const { getFramesForMetaStruct } = require('../../../src/appsec/stack_trace') - - // const list = getFramesForMetaStruct(32, getCallSiteInfo) - // const firstNonDDPath = pathLine.getNonDDFrames(list)[0] - - // const nodeModulesPaths = pathLine.getNodeModulesPaths(__filename) - - // expect(nodeModulesPaths[0]).to.eq(path.join('node_modules', process.cwd(), firstNonDDPath.path)) - - // pathLine.ddBasePath = basePath - // }) + function getCallSiteInfo () { + const previousPrepareStackTrace = Error.prepareStackTrace + const previousStackTraceLimit = Error.stackTraceLimit + let callsiteList + Error.stackTraceLimit = 100 + Error.prepareStackTrace = function (_, callsites) { + callsiteList = callsites + } + const e = new Error() + e.stack + Error.prepareStackTrace = previousPrepareStackTrace + Error.stackTraceLimit = previousStackTraceLimit + + return callsiteList + } + + it('should handle windows paths correctly', () => { + const basePath = pathLine.ddBasePath + pathLine.ddBasePath = path.join('test', 'base', 'path') + const { getCallsiteFrames } = require('../../../src/appsec/stack_trace') + + const list = getCallsiteFrames(32, getCallSiteInfo) + const firstNonDDPath = pathLine.getNonDDCallSiteFrames(list)[0] + + const expectedPath = path.join('node_modules', firstNonDDPath.path) + const nodeModulesPaths = pathLine.getNodeModulesPaths(firstNonDDPath.path) + + expect(nodeModulesPaths[0]).to.equal(expectedPath) + + pathLine.ddBasePath = basePath + }) it('should convert / to \\ in windows platforms', () => { const dirname = __dirname diff --git a/packages/dd-trace/test/appsec/stack_trace.spec.js b/packages/dd-trace/test/appsec/stack_trace.spec.js index 0808c2b32d3..7a5088c6070 100644 --- a/packages/dd-trace/test/appsec/stack_trace.spec.js +++ b/packages/dd-trace/test/appsec/stack_trace.spec.js @@ -3,7 +3,7 @@ const { assert } = require('chai') const path = require('path') -const { reportStackTrace, getFramesForMetaStruct } = require('../../src/appsec/stack_trace') +const { reportStackTrace, getCallsiteFrames } = require('../../src/appsec/stack_trace') describe('Stack trace reporter', () => { describe('frame filtering', () => { @@ -67,7 +67,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 const maxStackTraces = 2 - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, maxStackTraces, frames) @@ -114,7 +114,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) @@ -143,7 +143,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) @@ -176,7 +176,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) @@ -198,7 +198,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) @@ -218,7 +218,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 0, frames) @@ -238,7 +238,7 @@ describe('Stack trace reporter', () => { const stackId = 'test_stack_id' const maxDepth = 32 - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, -1, frames) @@ -289,7 +289,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) @@ -340,7 +340,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteListWithLibraryFrames) + const frames = getCallsiteFrames(maxDepth, () => callSiteListWithLibraryFrames) reportStackTrace(rootSpan, stackId, 2, frames) @@ -363,7 +363,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) @@ -386,7 +386,7 @@ describe('Stack trace reporter', () => { } )) - const frames = getFramesForMetaStruct(maxDepth, () => callSiteList) + const frames = getCallsiteFrames(maxDepth, () => callSiteList) reportStackTrace(rootSpan, stackId, 2, frames) From 18fc19cdc251da068baa1a698ae0bea4f1cb7cbf Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 15 Jan 2025 10:31:52 +0100 Subject: [PATCH 08/14] clear tests --- .../iast/analyzers/vulnerability-analyzer.spec.js | 10 +--------- .../dd-trace/test/appsec/iast/path-line.spec.js | 6 +++--- packages/dd-trace/test/appsec/iast/utils.js | 12 +----------- packages/dd-trace/test/appsec/rasp/utils.js | 13 +------------ packages/dd-trace/test/appsec/rasp/utils.spec.js | 3 +-- packages/dd-trace/test/appsec/stack_trace.spec.js | 6 ++---- packages/dd-trace/test/appsec/utils.js | 14 ++++++++++++++ 7 files changed, 23 insertions(+), 41 deletions(-) create mode 100644 packages/dd-trace/test/appsec/utils.js diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index c43c37b9da6..d26d67821b5 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -6,7 +6,6 @@ const proxyquire = require('proxyquire') describe('vulnerability-analyzer', () => { const VULNERABLE_VALUE = 'VULNERABLE_VALUE' const VULNERABILITY = 'VULNERABILITY' - const VULNERABILITY_LOCATION = { path: 'VULNERABILITY_LOCATION', line: 11 } const VULNERABILITY_LOCATION_FROM_SOURCEMAP = { path: 'VULNERABILITY_LOCATION_FROM_SOURCEMAP', line: 42, column: 21 } const ANALYZER_TYPE = 'TEST_ANALYZER' const SPAN_ID = '123456' @@ -14,18 +13,13 @@ describe('vulnerability-analyzer', () => { let VulnerabilityAnalyzer let vulnerabilityReporter let overheadController - let pathLine let iastContextHandler let rewriter beforeEach(() => { vulnerabilityReporter = { createVulnerability: sinon.stub().returns(VULNERABILITY), - addVulnerability: sinon.stub(), - getVulnerabilityCallSiteList: sinon.stub().returns([]) - } - pathLine = { - getNonDDPathAndLineFromCallsites: sinon.stub().returns([VULNERABILITY_LOCATION]) + addVulnerability: sinon.stub() } overheadController = { hasQuota: sinon.stub() @@ -39,7 +33,6 @@ describe('vulnerability-analyzer', () => { VulnerabilityAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/vulnerability-analyzer', { '../vulnerability-reporter': vulnerabilityReporter, - '../path-line': pathLine, '../overhead-controller': overheadController, '../iast-context': iastContextHandler, '../taint-tracking/rewriter': rewriter @@ -163,7 +156,6 @@ describe('vulnerability-analyzer', () => { VulnerabilityAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/vulnerability-analyzer', { '../vulnerability-reporter': vulnerabilityReporter, - '../path-line': pathLine, '../overhead-controller': overheadController, '../iast-context': iastContextHandler, '../iast-plugin': { diff --git a/packages/dd-trace/test/appsec/iast/path-line.spec.js b/packages/dd-trace/test/appsec/iast/path-line.spec.js index 11555a11e92..eee98c31ef9 100644 --- a/packages/dd-trace/test/appsec/iast/path-line.spec.js +++ b/packages/dd-trace/test/appsec/iast/path-line.spec.js @@ -2,6 +2,7 @@ const proxyquire = require('proxyquire') const path = require('path') const os = require('os') const { expect } = require('chai') +const { getCallsiteFrames } = require('../../../src/appsec/stack_trace') class CallSiteMock { constructor (fileName, lineNumber, columnNumber = 0) { @@ -97,8 +98,8 @@ describe('path-line', function () { it('should return all no DD entries when multiple stack frames are present', () => { const callsites = [] const expectedFilePaths = [ - path.join('first', 'file', 'out', 'of', 'dd1.js'), - path.join('second', 'file', 'out', 'of', 'dd2.js') + path.join('first', 'file', 'out', 'of', 'dd.js'), + path.join('second', 'file', 'out', 'of', 'dd.js') ] const firstFileOutOfDD = path.join(PROJECT_PATH, expectedFilePaths[0]) const secondFileOutOfDD = path.join(PROJECT_PATH, expectedFilePaths[1]) @@ -218,7 +219,6 @@ describe('path-line', function () { it('should handle windows paths correctly', () => { const basePath = pathLine.ddBasePath pathLine.ddBasePath = path.join('test', 'base', 'path') - const { getCallsiteFrames } = require('../../../src/appsec/stack_trace') const list = getCallsiteFrames(32, getCallSiteInfo) const firstNonDDPath = pathLine.getNonDDCallSiteFrames(list)[0] diff --git a/packages/dd-trace/test/appsec/iast/utils.js b/packages/dd-trace/test/appsec/iast/utils.js index ee6abe13cba..5597788bd9d 100644 --- a/packages/dd-trace/test/appsec/iast/utils.js +++ b/packages/dd-trace/test/appsec/iast/utils.js @@ -10,17 +10,7 @@ const axios = require('axios') const iast = require('../../../src/appsec/iast') const Config = require('../../../src/config') const vulnerabilityReporter = require('../../../src/appsec/iast/vulnerability-reporter') - -function getWebSpan (traces) { - for (const trace of traces) { - for (const span of trace) { - if (span.type === 'web') { - return span - } - } - } - throw new Error('web span not found') -} +const { getWebSpan } = require('../utils') function testInRequest (app, tests) { let http diff --git a/packages/dd-trace/test/appsec/rasp/utils.js b/packages/dd-trace/test/appsec/rasp/utils.js index 0d8a3e076a4..b8834afb468 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.js +++ b/packages/dd-trace/test/appsec/rasp/utils.js @@ -1,17 +1,7 @@ 'use strict' const { assert } = require('chai') - -function getWebSpan (traces) { - for (const trace of traces) { - for (const span of trace) { - if (span.type === 'web') { - return span - } - } - } - throw new Error('web span not found') -} +const { getWebSpan } = require('../utils') function checkRaspExecutedAndNotThreat (agent, checkRuleEval = true) { return agent.use((traces) => { @@ -39,7 +29,6 @@ function checkRaspExecutedAndHasThreat (agent, ruleId, ruleEvalCount = 1) { } module.exports = { - getWebSpan, checkRaspExecutedAndNotThreat, checkRaspExecutedAndHasThreat } diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js index 0389fe163d2..cec963b9e42 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.spec.js +++ b/packages/dd-trace/test/appsec/rasp/utils.spec.js @@ -11,8 +11,7 @@ describe('RASP - utils.js', () => { } stackTrace = { - reportStackTrace: sinon.stub(), - getCallSiteList: sinon.stub().returns([]) + reportStackTrace: sinon.stub() } utils = proxyquire('../../../src/appsec/rasp/utils', { diff --git a/packages/dd-trace/test/appsec/stack_trace.spec.js b/packages/dd-trace/test/appsec/stack_trace.spec.js index 7a5088c6070..08c7343a0a8 100644 --- a/packages/dd-trace/test/appsec/stack_trace.spec.js +++ b/packages/dd-trace/test/appsec/stack_trace.spec.js @@ -90,9 +90,8 @@ describe('Stack trace reporter', () => { it('should not fail if no root span is passed', () => { const rootSpan = undefined const stackId = 'test_stack_id' - const maxDepth = 32 try { - reportStackTrace(rootSpan, stackId, maxDepth, 2, callSiteList) + reportStackTrace(rootSpan, stackId, 2, callSiteList) } catch (e) { assert.fail() } @@ -253,9 +252,8 @@ describe('Stack trace reporter', () => { } } const stackId = 'test_stack_id' - const maxDepth = 32 const maxStackTraces = 2 - reportStackTrace(rootSpan, stackId, maxDepth, maxStackTraces, undefined) + reportStackTrace(rootSpan, stackId, maxStackTraces, undefined) assert.property(rootSpan.meta_struct, 'another_tag') assert.notProperty(rootSpan.meta_struct, '_dd.stack') }) diff --git a/packages/dd-trace/test/appsec/utils.js b/packages/dd-trace/test/appsec/utils.js new file mode 100644 index 00000000000..b881616b4cf --- /dev/null +++ b/packages/dd-trace/test/appsec/utils.js @@ -0,0 +1,14 @@ +function getWebSpan (traces) { + for (const trace of traces) { + for (const span of trace) { + if (span.type === 'web') { + return span + } + } + } + throw new Error('web span not found') +} + +module.exports = { + getWebSpan +} From 50ee6e2def8137d6eb6f4e7a5a445d139dfa043a Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 16 Jan 2025 15:00:06 +0100 Subject: [PATCH 09/14] get original locations only if we can add vulnerability --- .../iast/analyzers/vulnerability-analyzer.js | 10 ++- .../src/appsec/iast/vulnerability-reporter.js | 74 ++++++++++-------- .../iast/vulnerability-reporter.spec.js | 77 ++++++++++++++++++- 3 files changed, 123 insertions(+), 38 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index d5dcc96ed51..76de036a88d 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -2,7 +2,7 @@ const { storage } = require('../../../../../datadog-core') const { getNonDDCallSiteFrames } = require('../path-line') -const { addVulnerability, getVulnerabilityCallSiteFrames } = require('../vulnerability-reporter') +const { addVulnerability, canAddVulnerability, getVulnerabilityCallSiteFrames } = require('../vulnerability-reporter') const { getIastContext, getIastStackTraceId } = require('../iast-context') const overheadController = require('../overhead-controller') const { SinkIastPlugin } = require('../iast-plugin') @@ -34,8 +34,6 @@ class Analyzer extends SinkIastPlugin { const location = this._getLocation(value, nonDDCallSiteFrames) if (!this._isExcluded(location)) { - const originalCallSiteList = nonDDCallSiteFrames.map(callsite => this._replaceCallsiteFromSourceMap(callsite)) - const originalLocation = this._getOriginalLocation(location) const spanId = context && context.rootSpan && context.rootSpan.context().toSpanId() const stackId = getIastStackTraceId(context) @@ -47,7 +45,11 @@ class Analyzer extends SinkIastPlugin { stackId ) - addVulnerability(context, vulnerability, originalCallSiteList, stackId) + if (canAddVulnerability(vulnerability)) { + const originalCallSiteList = nonDDCallSiteFrames.map(callsite => this._replaceCallsiteFromSourceMap(callsite)) + + addVulnerability(context, vulnerability, originalCallSiteList, stackId) + } } } diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index cfdc2ee49cd..0b5190ba6fa 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -19,46 +19,53 @@ let deduplicationEnabled = true let stackTraceMaxDepth let maxStackTraces +function canAddVulnerability (vulnerability) { + const hasRequiredFields = vulnerability?.evidence && vulnerability?.type && vulnerability?.location + if (!hasRequiredFields) return false + + const isDuplicated = deduplicationEnabled && isDuplicatedVulnerability(vulnerability) + + return !isDuplicated +} + function addVulnerability (iastContext, vulnerability, callSiteFrames, stackId) { - if (vulnerability?.evidence && vulnerability?.type && vulnerability?.location) { - if (deduplicationEnabled && isDuplicatedVulnerability(vulnerability)) return + if (!canAddVulnerability(vulnerability)) return - VULNERABILITY_HASHES.set(`${vulnerability.type}${vulnerability.hash}`, true) + VULNERABILITY_HASHES.set(`${vulnerability.type}${vulnerability.hash}`, true) - let span = iastContext?.rootSpan + let span = iastContext?.rootSpan - if (!span && tracer) { - span = tracer.startSpan('vulnerability', { - type: 'vulnerability' - }) + if (!span && tracer) { + span = tracer.startSpan('vulnerability', { + type: 'vulnerability' + }) - vulnerability.location.spanId = span.context().toSpanId() + vulnerability.location.spanId = span.context().toSpanId() - span.addTags({ - [IAST_ENABLED_TAG_KEY]: 1 - }) - } + span.addTags({ + [IAST_ENABLED_TAG_KEY]: 1 + }) + } - if (!span) return - - keepTrace(span, SAMPLING_MECHANISM_APPSEC) - standalone.sample(span) - - reportStackTrace( - iastContext?.rootSpan, - stackId, - maxStackTraces, - callSiteFrames, - STACK_TRACE_NAMESPACES.IAST - ) - - if (iastContext?.rootSpan) { - iastContext[VULNERABILITIES_KEY] = iastContext[VULNERABILITIES_KEY] || [] - iastContext[VULNERABILITIES_KEY].push(vulnerability) - } else { - sendVulnerabilities([vulnerability], span) - span.finish() - } + if (!span) return + + keepTrace(span, SAMPLING_MECHANISM_APPSEC) + standalone.sample(span) + + reportStackTrace( + iastContext?.rootSpan, + stackId, + maxStackTraces, + callSiteFrames, + STACK_TRACE_NAMESPACES.IAST + ) + + if (iastContext?.rootSpan) { + iastContext[VULNERABILITIES_KEY] = iastContext[VULNERABILITIES_KEY] || [] + iastContext[VULNERABILITIES_KEY].push(vulnerability) + } else { + sendVulnerabilities([vulnerability], span) + span.finish() } } @@ -131,6 +138,7 @@ function stop () { module.exports = { addVulnerability, + canAddVulnerability, sendVulnerabilities, getVulnerabilityCallSiteFrames, clearCache, diff --git a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js index 3798882c727..872cdd4ccd6 100644 --- a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js +++ b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js @@ -1,4 +1,4 @@ -const { addVulnerability, sendVulnerabilities, clearCache, start, stop } = +const { addVulnerability, canAddVulnerability, sendVulnerabilities, clearCache, start, stop } = require('../../../src/appsec/iast/vulnerability-reporter') const VulnerabilityAnalyzer = require('../../../../dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer') const appsecStandalone = require('../../../src/appsec/standalone') @@ -465,6 +465,81 @@ describe('vulnerability-reporter', () => { }) }) + describe('canAddVulnerability', () => { + let vulnerability + + beforeEach(() => { + vulnerability = + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, undefined, + { path: 'filename.js', line: 73 }, 1) + + clearCache() + }) + + it('should return false when vulnerability is not defined', () => { + expect(canAddVulnerability(null)).to.be.false + }) + + it('should return false when vulnerability is missing required fields', () => { + const incompleteVulnerabilities = [ + { evidence: 'test', type: 'SQL_INJECTION' }, + { evidence: 'test', location: { line: 1 } }, + { type: 'SQL_INJECTION', location: { line: 1 } }, + {} + ] + + incompleteVulnerabilities.forEach(vulnerability => { + expect(canAddVulnerability(vulnerability)).to.be.false + }) + }) + + it('should return true for valid vulnerability when not duplicated', () => { + expect(canAddVulnerability(vulnerability)).to.be.true + }) + + it('should return false for duplicated vulnerability when deduplication is enabled', () => { + start({ + iast: { + deduplicationEnabled: true + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } + } + }) + + addVulnerability({ rootSpan: true }, vulnerability) + + expect(canAddVulnerability(vulnerability)).to.be.false + }) + + it('should return true for duplicated vulnerability when deduplication is disabled', () => { + start({ + iast: { + deduplicationEnabled: false + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } + } + }) + + addVulnerability({ rootSpan: true }, vulnerability) + + expect(canAddVulnerability(vulnerability)).to.be.true + }) + + afterEach(() => { + clearCache() + }) + }) + describe('clearCache', () => { let span let originalSetInterval From 91348189e780d513d1c446521be10e7abd17f276 Mon Sep 17 00:00:00 2001 From: ishabi Date: Fri, 17 Jan 2025 11:53:43 +0100 Subject: [PATCH 10/14] add iast stacktrace variable --- docs/test.ts | 5 ++- index.d.ts | 12 ++++++- .../src/appsec/iast/vulnerability-reporter.js | 18 ++++++---- packages/dd-trace/src/config.js | 4 +++ packages/dd-trace/test/config.spec.js | 34 ++++++++++++++++--- 5 files changed, 59 insertions(+), 14 deletions(-) diff --git a/docs/test.ts b/docs/test.ts index 2c2cbea332e..c353e90b6ca 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -136,7 +136,10 @@ tracer.init({ redactionEnabled: true, redactionNamePattern: 'password', redactionValuePattern: 'bearer', - telemetryVerbosity: 'OFF' + telemetryVerbosity: 'OFF', + stackTrace: { + enabled: true + } } }); diff --git a/index.d.ts b/index.d.ts index 8984d02f81a..8d3fdf24ded 100644 --- a/index.d.ts +++ b/index.d.ts @@ -2233,7 +2233,17 @@ declare namespace tracer { /** * Specifies the verbosity of the sent telemetry. Default 'INFORMATION' */ - telemetryVerbosity?: string + telemetryVerbosity?: string, + + /** + * Configuration for stack trace reporting + */ + stackTrace?: { + /** Whether to enable stack trace reporting. + * @default true + */ + enabled?: boolean, + } } export namespace llmobs { diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 0b5190ba6fa..2f7d097d94b 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -16,6 +16,7 @@ const RESET_VULNERABILITY_CACHE_INTERVAL = 60 * 60 * 1000 // 1 hour let tracer let resetVulnerabilityCacheTimer let deduplicationEnabled = true +let stackTraceEnabled = true let stackTraceMaxDepth let maxStackTraces @@ -52,13 +53,15 @@ function addVulnerability (iastContext, vulnerability, callSiteFrames, stackId) keepTrace(span, SAMPLING_MECHANISM_APPSEC) standalone.sample(span) - reportStackTrace( - iastContext?.rootSpan, - stackId, - maxStackTraces, - callSiteFrames, - STACK_TRACE_NAMESPACES.IAST - ) + if (stackTraceEnabled) { + reportStackTrace( + iastContext?.rootSpan, + stackId, + maxStackTraces, + callSiteFrames, + STACK_TRACE_NAMESPACES.IAST + ) + } if (iastContext?.rootSpan) { iastContext[VULNERABILITIES_KEY] = iastContext[VULNERABILITIES_KEY] || [] @@ -118,6 +121,7 @@ function getVulnerabilityCallSiteFrames () { function start (config, _tracer) { deduplicationEnabled = config.iast.deduplicationEnabled + stackTraceEnabled = config.iast.stackTrace.enabled stackTraceMaxDepth = config.appsec.stackTrace.maxDepth maxStackTraces = config.appsec.stackTrace.maxStackTraces diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 8dd63cccdf6..f529bc635e2 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -497,6 +497,7 @@ class Config { this._setValue(defaults, 'iast.redactionValuePattern', null) this._setValue(defaults, 'iast.requestSampling', 30) this._setValue(defaults, 'iast.telemetryVerbosity', 'INFORMATION') + this._setValue(defaults, 'iast.stackTrace.enabled', true) this._setValue(defaults, 'injectionEnabled', []) this._setValue(defaults, 'isAzureFunction', false) this._setValue(defaults, 'isCiVisibility', false) @@ -622,6 +623,7 @@ class Config { DD_IAST_REDACTION_VALUE_PATTERN, DD_IAST_REQUEST_SAMPLING, DD_IAST_TELEMETRY_VERBOSITY, + DD_IAST_STACK_TRACE_ENABLED, DD_INJECTION_ENABLED, DD_INSTRUMENTATION_TELEMETRY_ENABLED, DD_INSTRUMENTATION_CONFIG_ID, @@ -787,6 +789,7 @@ class Config { } this._envUnprocessed['iast.requestSampling'] = DD_IAST_REQUEST_SAMPLING this._setString(env, 'iast.telemetryVerbosity', DD_IAST_TELEMETRY_VERBOSITY) + this._setBoolean(env, 'iast.stackTrace.enabled', DD_IAST_STACK_TRACE_ENABLED) this._setArray(env, 'injectionEnabled', DD_INJECTION_ENABLED) this._setBoolean(env, 'isAzureFunction', getIsAzureFunction()) this._setBoolean(env, 'isGCPFunction', getIsGCPFunction()) @@ -976,6 +979,7 @@ class Config { this._optsUnprocessed['iast.requestSampling'] = options.iast?.requestSampling } this._setString(opts, 'iast.telemetryVerbosity', options.iast && options.iast.telemetryVerbosity) + this._setBoolean(opts, 'iast.stackTrace.enabled', options.iast?.stackTrace?.enabled) this._setBoolean(opts, 'isCiVisibility', options.isCiVisibility) this._setBoolean(opts, 'legacyBaggageEnabled', options.legacyBaggageEnabled) this._setBoolean(opts, 'llmobs.agentlessEnabled', options.llmobs?.agentlessEnabled) diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index 6bf7bf32e98..1b43a7859b2 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -265,6 +265,7 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionNamePattern', null) expect(config).to.have.nested.property('iast.redactionValuePattern', null) expect(config).to.have.nested.property('iast.telemetryVerbosity', 'INFORMATION') + expect(config).to.have.nested.property('iast.stackTrace.enabled', true) expect(config).to.have.nested.property('installSignature.id', null) expect(config).to.have.nested.property('installSignature.time', null) expect(config).to.have.nested.property('installSignature.type', null) @@ -330,6 +331,7 @@ describe('Config', () => { { name: 'iast.redactionValuePattern', value: null, origin: 'default' }, { name: 'iast.requestSampling', value: 30, origin: 'default' }, { name: 'iast.telemetryVerbosity', value: 'INFORMATION', origin: 'default' }, + { name: 'iast.stackTrace.enabled', value: true, origin: 'default' }, { name: 'injectionEnabled', value: [], origin: 'default' }, { name: 'isCiVisibility', value: false, origin: 'default' }, { name: 'isEarlyFlakeDetectionEnabled', value: false, origin: 'default' }, @@ -509,6 +511,7 @@ describe('Config', () => { process.env.DD_IAST_REDACTION_NAME_PATTERN = 'REDACTION_NAME_PATTERN' process.env.DD_IAST_REDACTION_VALUE_PATTERN = 'REDACTION_VALUE_PATTERN' process.env.DD_IAST_TELEMETRY_VERBOSITY = 'DEBUG' + process.env.DD_IAST_STACK_TRACE_ENABLED = 'false' process.env.DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED = 'true' process.env.DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = 'true' process.env.DD_PROFILING_ENABLED = 'true' @@ -623,6 +626,7 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionNamePattern', 'REDACTION_NAME_PATTERN') expect(config).to.have.nested.property('iast.redactionValuePattern', 'REDACTION_VALUE_PATTERN') expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') + expect(config).to.have.nested.property('iast.stackTrace.enabled', false) expect(config).to.have.deep.property('installSignature', { id: '68e75c48-57ca-4a12-adfc-575c4b05fcbe', type: 'k8s_single_step', @@ -674,6 +678,7 @@ describe('Config', () => { { name: 'iast.redactionValuePattern', value: 'REDACTION_VALUE_PATTERN', origin: 'env_var' }, { name: 'iast.requestSampling', value: '40', origin: 'env_var' }, { name: 'iast.telemetryVerbosity', value: 'DEBUG', origin: 'env_var' }, + { name: 'iast.stackTrace.enabled', value: false, origin: 'env_var' }, { name: 'instrumentation_config_id', value: 'abcdef123', origin: 'env_var' }, { name: 'injectionEnabled', value: ['profiler'], origin: 'env_var' }, { name: 'isGCPFunction', value: false, origin: 'env_var' }, @@ -872,7 +877,10 @@ describe('Config', () => { redactionEnabled: false, redactionNamePattern: 'REDACTION_NAME_PATTERN', redactionValuePattern: 'REDACTION_VALUE_PATTERN', - telemetryVerbosity: 'DEBUG' + telemetryVerbosity: 'DEBUG', + stackTrace: { + enabled: false + } }, appsec: { standalone: { @@ -948,6 +956,7 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionNamePattern', 'REDACTION_NAME_PATTERN') expect(config).to.have.nested.property('iast.redactionValuePattern', 'REDACTION_VALUE_PATTERN') expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') + expect(config).to.have.nested.property('iast.stackTrace.enabled', false) expect(config).to.have.deep.nested.property('sampler', { sampleRate: 0.5, rateLimit: 1000, @@ -1002,6 +1011,7 @@ describe('Config', () => { { name: 'iast.redactionValuePattern', value: 'REDACTION_VALUE_PATTERN', origin: 'code' }, { name: 'iast.requestSampling', value: 50, origin: 'code' }, { name: 'iast.telemetryVerbosity', value: 'DEBUG', origin: 'code' }, + { name: 'iast.stackTrace.enabled', value: false, origin: 'code' }, { name: 'peerServiceMapping', value: { d: 'dd' }, origin: 'code' }, { name: 'plugins', value: false, origin: 'code' }, { name: 'port', value: '6218', origin: 'code' }, @@ -1224,6 +1234,7 @@ describe('Config', () => { process.env.DD_IAST_COOKIE_FILTER_PATTERN = '.*' process.env.DD_IAST_REDACTION_NAME_PATTERN = 'name_pattern_to_be_overriden_by_options' process.env.DD_IAST_REDACTION_VALUE_PATTERN = 'value_pattern_to_be_overriden_by_options' + process.env.DD_IAST_STACK_TRACE_ENABLED = 'true' process.env.DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED = 'true' process.env.DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = 'true' process.env.DD_LLMOBS_ML_APP = 'myMlApp' @@ -1304,7 +1315,10 @@ describe('Config', () => { cookieFilterPattern: '.{10,}', dbRowsToTaint: 3, redactionNamePattern: 'REDACTION_NAME_PATTERN', - redactionValuePattern: 'REDACTION_VALUE_PATTERN' + redactionValuePattern: 'REDACTION_VALUE_PATTERN', + stackTrace: { + enabled: false + } }, remoteConfig: { pollInterval: 42 @@ -1379,6 +1393,7 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionEnabled', true) expect(config).to.have.nested.property('iast.redactionNamePattern', 'REDACTION_NAME_PATTERN') expect(config).to.have.nested.property('iast.redactionValuePattern', 'REDACTION_VALUE_PATTERN') + expect(config).to.have.nested.property('iast.stackTrace.enabled', false) expect(config).to.have.nested.property('llmobs.mlApp', 'myOtherMlApp') expect(config).to.have.nested.property('llmobs.agentlessEnabled', false) }) @@ -1416,7 +1431,10 @@ describe('Config', () => { redactionEnabled: false, redactionNamePattern: 'REDACTION_NAME_PATTERN', redactionValuePattern: 'REDACTION_VALUE_PATTERN', - telemetryVerbosity: 'DEBUG' + telemetryVerbosity: 'DEBUG', + stackTrace: { + enabled: false + } }, experimental: { appsec: { @@ -1450,7 +1468,10 @@ describe('Config', () => { redactionEnabled: true, redactionNamePattern: 'IGNORED_REDACTION_NAME_PATTERN', redactionValuePattern: 'IGNORED_REDACTION_VALUE_PATTERN', - telemetryVerbosity: 'OFF' + telemetryVerbosity: 'OFF', + stackTrace: { + enabled: true + } } } }) @@ -1499,7 +1520,10 @@ describe('Config', () => { redactionEnabled: false, redactionNamePattern: 'REDACTION_NAME_PATTERN', redactionValuePattern: 'REDACTION_VALUE_PATTERN', - telemetryVerbosity: 'DEBUG' + telemetryVerbosity: 'DEBUG', + stackTrace: { + enabled: false + } }) }) From ed76f35407ae17831c93a47c164de2eaa8ac1170 Mon Sep 17 00:00:00 2001 From: ishabi Date: Fri, 17 Jan 2025 14:44:24 +0100 Subject: [PATCH 11/14] vulnerability reporter unit test with stack trace --- .../iast/vulnerability-reporter.spec.js | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js index 872cdd4ccd6..034e5d4894b 100644 --- a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js +++ b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js @@ -106,7 +106,10 @@ describe('vulnerability-reporter', () => { } start({ iast: { - deduplicationEnabled: true + deduplicationEnabled: true, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -168,7 +171,10 @@ describe('vulnerability-reporter', () => { } start({ iast: { - deduplicationEnabled: true + deduplicationEnabled: true, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -399,7 +405,10 @@ describe('vulnerability-reporter', () => { it('should not deduplicate vulnerabilities if not enabled', () => { start({ iast: { - deduplicationEnabled: false + deduplicationEnabled: false, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -500,7 +509,10 @@ describe('vulnerability-reporter', () => { it('should return false for duplicated vulnerability when deduplication is enabled', () => { start({ iast: { - deduplicationEnabled: true + deduplicationEnabled: true, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -519,7 +531,10 @@ describe('vulnerability-reporter', () => { it('should return true for duplicated vulnerability when deduplication is disabled', () => { start({ iast: { - deduplicationEnabled: false + deduplicationEnabled: false, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -592,7 +607,10 @@ describe('vulnerability-reporter', () => { it('should set timer to clear cache every hour if deduplication is enabled', () => { const config = { iast: { - deduplicationEnabled: true + deduplicationEnabled: true, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -609,7 +627,10 @@ describe('vulnerability-reporter', () => { it('should not set timer to clear cache every hour if deduplication is not enabled', () => { const config = { iast: { - deduplicationEnabled: false + deduplicationEnabled: false, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -626,7 +647,10 @@ describe('vulnerability-reporter', () => { it('should unset timer to clear cache every hour', () => { const config = { iast: { - deduplicationEnabled: true + deduplicationEnabled: true, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { @@ -658,7 +682,10 @@ describe('vulnerability-reporter', () => { iast: { redactionEnabled: true, redactionNamePattern: null, - redactionValuePattern: null + redactionValuePattern: null, + stackTrace: { + enabled: true + } }, appsec: { stackTrace: { From 9af112c08473e690918f8a0ee3913a64c1fc2b97 Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 23 Jan 2025 09:23:42 +0100 Subject: [PATCH 12/14] maintain stack trace limit per request --- .../iast/analyzers/vulnerability-analyzer.js | 8 ++-- .../src/appsec/iast/vulnerability-reporter.js | 9 ++-- packages/dd-trace/src/appsec/rasp/utils.js | 15 ++++-- packages/dd-trace/src/appsec/stack_trace.js | 36 +++++++-------- .../analyzers/vulnerability-analyzer.spec.js | 3 +- .../dd-trace/test/appsec/rasp/utils.spec.js | 37 ++++++++++++++- .../dd-trace/test/appsec/stack_trace.spec.js | 46 +++++-------------- 7 files changed, 83 insertions(+), 71 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index 76de036a88d..256e41d9880 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -46,9 +46,9 @@ class Analyzer extends SinkIastPlugin { ) if (canAddVulnerability(vulnerability)) { - const originalCallSiteList = nonDDCallSiteFrames.map(callsite => this._replaceCallsiteFromSourceMap(callsite)) + const originalCallSiteList = nonDDCallSiteFrames.map(callsite => this._replaceCallSiteFromSourceMap(callsite)) - addVulnerability(context, vulnerability, originalCallSiteList, stackId) + addVulnerability(context, vulnerability, originalCallSiteList) } } } @@ -70,7 +70,7 @@ class Analyzer extends SinkIastPlugin { } _getOriginalLocation (location) { - const locationFromSourceMap = this._replaceCallsiteFromSourceMap(location) + const locationFromSourceMap = this._replaceCallSiteFromSourceMap(location) const originalLocation = {} if (locationFromSourceMap?.path) { @@ -86,7 +86,7 @@ class Analyzer extends SinkIastPlugin { return originalLocation } - _replaceCallsiteFromSourceMap (callsite) { + _replaceCallSiteFromSourceMap (callsite) { if (callsite) { const { path, line, column } = getOriginalPathAndLineFromSourceMap(callsite) if (path) { diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 2f7d097d94b..75a34659c29 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -18,7 +18,6 @@ let resetVulnerabilityCacheTimer let deduplicationEnabled = true let stackTraceEnabled = true let stackTraceMaxDepth -let maxStackTraces function canAddVulnerability (vulnerability) { const hasRequiredFields = vulnerability?.evidence && vulnerability?.type && vulnerability?.location @@ -29,7 +28,7 @@ function canAddVulnerability (vulnerability) { return !isDuplicated } -function addVulnerability (iastContext, vulnerability, callSiteFrames, stackId) { +function addVulnerability (iastContext, vulnerability, callSiteFrames) { if (!canAddVulnerability(vulnerability)) return VULNERABILITY_HASHES.set(`${vulnerability.type}${vulnerability.hash}`, true) @@ -55,9 +54,8 @@ function addVulnerability (iastContext, vulnerability, callSiteFrames, stackId) if (stackTraceEnabled) { reportStackTrace( - iastContext?.rootSpan, - stackId, - maxStackTraces, + span, + vulnerability.stackId, callSiteFrames, STACK_TRACE_NAMESPACES.IAST ) @@ -123,7 +121,6 @@ function start (config, _tracer) { deduplicationEnabled = config.iast.deduplicationEnabled stackTraceEnabled = config.iast.stackTrace.enabled stackTraceMaxDepth = config.appsec.stackTrace.maxDepth - maxStackTraces = config.appsec.stackTrace.maxStackTraces vulnerabilitiesFormatter.setRedactVulnerabilities( config.iast.redactionEnabled, diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index bf17b9414c2..f14aff3482b 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -1,7 +1,7 @@ 'use strict' const web = require('../../plugins/util/web') -const { getCallsiteFrames, reportStackTrace } = require('../stack_trace') +const { getCallsiteFrames, reportStackTrace, STACK_TRACE_NAMESPACES } = require('../stack_trace') const { getBlockingAction } = require('../blocking') const log = require('../../log') @@ -33,14 +33,14 @@ function handleResult (actions, req, res, abortController, config) { const { enabled, maxDepth, maxStackTraces } = config.appsec.stackTrace - if (generateStackTraceAction && enabled) { + const rootSpan = web.root(req) + + if (generateStackTraceAction && enabled && canReportStackTrace(rootSpan, maxStackTraces)) { const frames = getCallsiteFrames(maxDepth) - const rootSpan = web.root(req) reportStackTrace( rootSpan, generateStackTraceAction.stack_id, - maxStackTraces, frames ) } @@ -63,6 +63,13 @@ function handleResult (actions, req, res, abortController, config) { } } +function canReportStackTrace (rootSpan, maxStackTraces) { + if (!rootSpan) return false + + return maxStackTraces < 1 || + (rootSpan.meta_struct?.['_dd.stack']?.[STACK_TRACE_NAMESPACES.RASP]?.length ?? 0) < maxStackTraces +} + module.exports = { handleResult, RULE_TYPES, diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index 72e97cc8571..f57959698d6 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -63,31 +63,27 @@ function getCallsiteFrames (maxDepth = 32, callSiteListGetter = getCallSiteList) return indexedFrames } -function reportStackTrace ( - rootSpan, stackId, maxStackTraces, frames, namespace = STACK_TRACE_NAMESPACES.RASP) { +function reportStackTrace (rootSpan, stackId, frames, namespace = STACK_TRACE_NAMESPACES.RASP) { if (!rootSpan) return + if (!Array.isArray(frames)) return - if (maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.[namespace]?.length ?? 0) < maxStackTraces) { - if (!Array.isArray(frames)) return - - if (!rootSpan.meta_struct) { - rootSpan.meta_struct = {} - } - - if (!rootSpan.meta_struct['_dd.stack']) { - rootSpan.meta_struct['_dd.stack'] = {} - } + if (!rootSpan.meta_struct) { + rootSpan.meta_struct = {} + } - if (!rootSpan.meta_struct['_dd.stack'][namespace]) { - rootSpan.meta_struct['_dd.stack'][namespace] = [] - } + if (!rootSpan.meta_struct['_dd.stack']) { + rootSpan.meta_struct['_dd.stack'] = {} + } - rootSpan.meta_struct['_dd.stack'][namespace].push({ - id: stackId, - language: 'nodejs', - frames - }) + if (!rootSpan.meta_struct['_dd.stack'][namespace]) { + rootSpan.meta_struct['_dd.stack'][namespace] = [] } + + rootSpan.meta_struct['_dd.stack'][namespace].push({ + id: stackId, + language: 'nodejs', + frames + }) } module.exports = { diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index d26d67821b5..5e7220953f9 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -124,8 +124,7 @@ describe('vulnerability-analyzer', () => { }, hash: 5975567724 }, - sinon.match.array, - 1 + sinon.match.array ) }) diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js index cec963b9e42..6a74c07444d 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.spec.js +++ b/packages/dd-trace/test/appsec/rasp/utils.spec.js @@ -44,7 +44,42 @@ describe('RASP - utils.js', () => { web.root.returns(rootSpan) utils.handleResult(result, req, undefined, undefined, config) - sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, 2, sinon.match.array) + sinon.assert.calledOnceWithExactly(stackTrace.reportStackTrace, rootSpan, stackId, sinon.match.array) + }) + + it('should not report stack trace when max stack traces limit is reached', () => { + const req = {} + const rootSpan = { + meta_struct: { + '_dd.stack': { + exploit: ['stack1', 'stack2'] + } + } + } + const result = { + generate_stack: { + stack_id: 'stackId' + } + } + + web.root.returns(rootSpan) + + utils.handleResult(result, req, undefined, undefined, config) + sinon.assert.notCalled(stackTrace.reportStackTrace) + }) + + it('should not report stack trace when rootSpan is null', () => { + const req = {} + const result = { + generate_stack: { + stack_id: 'stackId' + } + } + + web.root.returns(null) + + utils.handleResult(result, req, undefined, undefined, config) + sinon.assert.notCalled(stackTrace.reportStackTrace) }) it('should not report stack trace when no action is present in waf result', () => { diff --git a/packages/dd-trace/test/appsec/stack_trace.spec.js b/packages/dd-trace/test/appsec/stack_trace.spec.js index 08c7343a0a8..406944c0381 100644 --- a/packages/dd-trace/test/appsec/stack_trace.spec.js +++ b/packages/dd-trace/test/appsec/stack_trace.spec.js @@ -66,10 +66,9 @@ describe('Stack trace reporter', () => { const rootSpan = {} const stackId = 'test_stack_id' const maxDepth = 32 - const maxStackTraces = 2 const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, maxStackTraces, frames) + reportStackTrace(rootSpan, stackId, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -91,7 +90,7 @@ describe('Stack trace reporter', () => { const rootSpan = undefined const stackId = 'test_stack_id' try { - reportStackTrace(rootSpan, stackId, 2, callSiteList) + reportStackTrace(rootSpan, stackId, callSiteList) } catch (e) { assert.fail() } @@ -115,7 +114,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].language, 'nodejs') @@ -144,7 +143,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].language, 'nodejs') @@ -177,7 +176,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[1].id, stackId) assert.strictEqual(rootSpan.meta_struct['_dd.stack'].exploit[1].language, 'nodejs') @@ -185,26 +184,6 @@ describe('Stack trace reporter', () => { assert.property(rootSpan.meta_struct, 'another_tag') }) - it('should not report stack trace when the maximum has been reached', () => { - const rootSpan = { - meta_struct: { - '_dd.stack': { - exploit: [callSiteList, callSiteList] - }, - another_tag: [] - } - } - const stackId = 'test_stack_id' - const maxDepth = 32 - - const frames = getCallsiteFrames(maxDepth, () => callSiteList) - - reportStackTrace(rootSpan, stackId, 2, frames) - - assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 2) - assert.property(rootSpan.meta_struct, 'another_tag') - }) - it('should add stack trace when the max stack trace is 0', () => { const rootSpan = { meta_struct: { @@ -219,7 +198,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 0, frames) + reportStackTrace(rootSpan, stackId, frames) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 3) assert.property(rootSpan.meta_struct, 'another_tag') @@ -239,7 +218,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, -1, frames) + reportStackTrace(rootSpan, stackId, frames) assert.equal(rootSpan.meta_struct['_dd.stack'].exploit.length, 3) assert.property(rootSpan.meta_struct, 'another_tag') @@ -252,8 +231,7 @@ describe('Stack trace reporter', () => { } } const stackId = 'test_stack_id' - const maxStackTraces = 2 - reportStackTrace(rootSpan, stackId, maxStackTraces, undefined) + reportStackTrace(rootSpan, stackId, undefined) assert.property(rootSpan.meta_struct, 'another_tag') assert.notProperty(rootSpan.meta_struct, '_dd.stack') }) @@ -289,7 +267,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -340,7 +318,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteListWithLibraryFrames) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -363,7 +341,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) @@ -386,7 +364,7 @@ describe('Stack trace reporter', () => { const frames = getCallsiteFrames(maxDepth, () => callSiteList) - reportStackTrace(rootSpan, stackId, 2, frames) + reportStackTrace(rootSpan, stackId, frames) assert.deepEqual(rootSpan.meta_struct['_dd.stack'].exploit[0].frames, expectedFrames) }) From 7877d99b38b10a9e39de7b5db53c2bc80448f7eb Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 23 Jan 2025 14:21:24 +0100 Subject: [PATCH 13/14] dont report stack trace if we reach max by request --- .../iast/analyzers/vulnerability-analyzer.js | 33 +-- .../src/appsec/iast/vulnerability-reporter.js | 31 ++- packages/dd-trace/src/appsec/rasp/utils.js | 9 +- packages/dd-trace/src/appsec/stack_trace.js | 7 + .../analyzers/vulnerability-analyzer.spec.js | 10 +- .../iast/vulnerability-reporter.spec.js | 241 ++++++++++-------- 6 files changed, 175 insertions(+), 156 deletions(-) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index 256e41d9880..1cb244dbbdc 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -2,11 +2,14 @@ const { storage } = require('../../../../../datadog-core') const { getNonDDCallSiteFrames } = require('../path-line') -const { addVulnerability, canAddVulnerability, getVulnerabilityCallSiteFrames } = require('../vulnerability-reporter') const { getIastContext, getIastStackTraceId } = require('../iast-context') const overheadController = require('../overhead-controller') const { SinkIastPlugin } = require('../iast-plugin') -const { getOriginalPathAndLineFromSourceMap } = require('../taint-tracking/rewriter') +const { + addVulnerability, + getVulnerabilityCallSiteFrames, + replaceCallSiteFromSourceMap +} = require('../vulnerability-reporter') class Analyzer extends SinkIastPlugin { constructor (type) { @@ -45,11 +48,7 @@ class Analyzer extends SinkIastPlugin { stackId ) - if (canAddVulnerability(vulnerability)) { - const originalCallSiteList = nonDDCallSiteFrames.map(callsite => this._replaceCallSiteFromSourceMap(callsite)) - - addVulnerability(context, vulnerability, originalCallSiteList) - } + addVulnerability(context, vulnerability, nonDDCallSiteFrames) } } @@ -70,7 +69,7 @@ class Analyzer extends SinkIastPlugin { } _getOriginalLocation (location) { - const locationFromSourceMap = this._replaceCallSiteFromSourceMap(location) + const locationFromSourceMap = replaceCallSiteFromSourceMap(location) const originalLocation = {} if (locationFromSourceMap?.path) { @@ -86,24 +85,6 @@ class Analyzer extends SinkIastPlugin { return originalLocation } - _replaceCallSiteFromSourceMap (callsite) { - if (callsite) { - const { path, line, column } = getOriginalPathAndLineFromSourceMap(callsite) - if (path) { - callsite.file = path - callsite.path = path - } - if (line) { - callsite.line = line - } - if (column) { - callsite.column = column - } - } - - return callsite - } - _getExcludedPaths () {} _isInvalidContext (store, iastContext) { diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index 75a34659c29..4adc636e5af 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -6,7 +6,8 @@ const { IAST_ENABLED_TAG_KEY, IAST_JSON_TAG_KEY } = require('./tags') const standalone = require('../standalone') const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') const { keepTrace } = require('../../priority_sampler') -const { reportStackTrace, getCallsiteFrames, STACK_TRACE_NAMESPACES } = require('../stack_trace') +const { reportStackTrace, getCallsiteFrames, canReportStackTrace, STACK_TRACE_NAMESPACES } = require('../stack_trace') +const { getOriginalPathAndLineFromSourceMap } = require('./taint-tracking/rewriter') const VULNERABILITIES_KEY = 'vulnerabilities' const VULNERABILITY_HASHES_MAX_SIZE = 1000 @@ -18,6 +19,7 @@ let resetVulnerabilityCacheTimer let deduplicationEnabled = true let stackTraceEnabled = true let stackTraceMaxDepth +let maxStackTraces function canAddVulnerability (vulnerability) { const hasRequiredFields = vulnerability?.evidence && vulnerability?.type && vulnerability?.location @@ -52,11 +54,13 @@ function addVulnerability (iastContext, vulnerability, callSiteFrames) { keepTrace(span, SAMPLING_MECHANISM_APPSEC) standalone.sample(span) - if (stackTraceEnabled) { + if (stackTraceEnabled && canReportStackTrace(span, maxStackTraces, STACK_TRACE_NAMESPACES.IAST)) { + const originalCallSiteList = callSiteFrames.map(callsite => replaceCallSiteFromSourceMap(callsite)) + reportStackTrace( span, vulnerability.stackId, - callSiteFrames, + originalCallSiteList, STACK_TRACE_NAMESPACES.IAST ) } @@ -117,10 +121,29 @@ function getVulnerabilityCallSiteFrames () { return getCallsiteFrames(stackTraceMaxDepth) } +function replaceCallSiteFromSourceMap (callsite) { + if (callsite) { + const { path, line, column } = getOriginalPathAndLineFromSourceMap(callsite) + if (path) { + callsite.file = path + callsite.path = path + } + if (line) { + callsite.line = line + } + if (column) { + callsite.column = column + } + } + + return callsite +} + function start (config, _tracer) { deduplicationEnabled = config.iast.deduplicationEnabled stackTraceEnabled = config.iast.stackTrace.enabled stackTraceMaxDepth = config.appsec.stackTrace.maxDepth + maxStackTraces = config.appsec.stackTrace.maxStackTraces vulnerabilitiesFormatter.setRedactVulnerabilities( config.iast.redactionEnabled, @@ -139,9 +162,9 @@ function stop () { module.exports = { addVulnerability, - canAddVulnerability, sendVulnerabilities, getVulnerabilityCallSiteFrames, + replaceCallSiteFromSourceMap, clearCache, start, stop diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index f14aff3482b..17875c48c7b 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -1,7 +1,7 @@ 'use strict' const web = require('../../plugins/util/web') -const { getCallsiteFrames, reportStackTrace, STACK_TRACE_NAMESPACES } = require('../stack_trace') +const { getCallsiteFrames, reportStackTrace, canReportStackTrace } = require('../stack_trace') const { getBlockingAction } = require('../blocking') const log = require('../../log') @@ -63,13 +63,6 @@ function handleResult (actions, req, res, abortController, config) { } } -function canReportStackTrace (rootSpan, maxStackTraces) { - if (!rootSpan) return false - - return maxStackTraces < 1 || - (rootSpan.meta_struct?.['_dd.stack']?.[STACK_TRACE_NAMESPACES.RASP]?.length ?? 0) < maxStackTraces -} - module.exports = { handleResult, RULE_TYPES, diff --git a/packages/dd-trace/src/appsec/stack_trace.js b/packages/dd-trace/src/appsec/stack_trace.js index f57959698d6..53fc0e27811 100644 --- a/packages/dd-trace/src/appsec/stack_trace.js +++ b/packages/dd-trace/src/appsec/stack_trace.js @@ -86,8 +86,15 @@ function reportStackTrace (rootSpan, stackId, frames, namespace = STACK_TRACE_NA }) } +function canReportStackTrace (rootSpan, maxStackTraces, namespace = STACK_TRACE_NAMESPACES.RASP) { + if (!rootSpan) return false + + return maxStackTraces < 1 || (rootSpan.meta_struct?.['_dd.stack']?.[namespace]?.length ?? 0) < maxStackTraces +} + module.exports = { getCallsiteFrames, reportStackTrace, + canReportStackTrace, STACK_TRACE_NAMESPACES } diff --git a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js index 5e7220953f9..cdb7e8cc4e2 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/vulnerability-analyzer.spec.js @@ -14,12 +14,12 @@ describe('vulnerability-analyzer', () => { let vulnerabilityReporter let overheadController let iastContextHandler - let rewriter beforeEach(() => { vulnerabilityReporter = { createVulnerability: sinon.stub().returns(VULNERABILITY), - addVulnerability: sinon.stub() + addVulnerability: sinon.stub(), + replaceCallSiteFromSourceMap: sinon.stub().returns(VULNERABILITY_LOCATION_FROM_SOURCEMAP) } overheadController = { hasQuota: sinon.stub() @@ -27,15 +27,11 @@ describe('vulnerability-analyzer', () => { iastContextHandler = { getIastContext: sinon.stub() } - rewriter = { - getOriginalPathAndLineFromSourceMap: sinon.stub().returns(VULNERABILITY_LOCATION_FROM_SOURCEMAP) - } VulnerabilityAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/vulnerability-analyzer', { '../vulnerability-reporter': vulnerabilityReporter, '../overhead-controller': overheadController, - '../iast-context': iastContextHandler, - '../taint-tracking/rewriter': rewriter + '../iast-context': iastContextHandler }) }) diff --git a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js index 034e5d4894b..9cf28bdac32 100644 --- a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js +++ b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js @@ -1,4 +1,4 @@ -const { addVulnerability, canAddVulnerability, sendVulnerabilities, clearCache, start, stop } = +const { addVulnerability, sendVulnerabilities, clearCache, start, stop } = require('../../../src/appsec/iast/vulnerability-reporter') const VulnerabilityAnalyzer = require('../../../../dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer') const appsecStandalone = require('../../../src/appsec/standalone') @@ -28,12 +28,12 @@ describe('vulnerability-reporter', () => { describe('with rootSpan', () => { let iastContext = { - rootSpan: true + rootSpan: {} } afterEach(() => { iastContext = { - rootSpan: true + rootSpan: {} } }) @@ -47,27 +47,27 @@ describe('vulnerability-reporter', () => { it('should create vulnerability array if it does not exist', () => { addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888), []) expect(iastContext).to.have.property('vulnerabilities') expect(iastContext.vulnerabilities).to.be.an('array') }) it('should deduplicate same vulnerabilities', () => { addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, -555)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, -555), []) addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888), []) addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 123)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 123), []) expect(iastContext.vulnerabilities).to.have.length(1) }) it('should add in the context evidence properties', () => { addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888), []) addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'md5' }, - -123, { path: 'path.js', line: 12 })) + -123, { path: 'path.js', line: 12 }), []) expect(iastContext.vulnerabilities).to.have.length(2) expect(iastContext).to.have.nested.property('vulnerabilities.0.type', 'INSECURE_HASHING') expect(iastContext).to.have.nested.property('vulnerabilities.0.evidence.value', 'sha1') @@ -130,7 +130,7 @@ describe('vulnerability-reporter', () => { const vulnerability = vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, undefined, { path: 'filename.js', line: 73 }, 1) - addVulnerability(undefined, vulnerability) + addVulnerability(undefined, vulnerability, []) expect(fakeTracer.startSpan).to.have.been.calledOnceWithExactly('vulnerability', { type: 'vulnerability' }) expect(onTheFlySpan.addTags.firstCall).to.have.been.calledWithExactly({ '_dd.iast.enabled': 1 @@ -148,12 +148,108 @@ describe('vulnerability-reporter', () => { const vulnerability = vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, undefined, { path: 'filename.js', line: 73 }) - addVulnerability(undefined, vulnerability) + addVulnerability(undefined, vulnerability, []) expect(vulnerability.location.spanId).to.be.equal(42) }) }) }) + describe('with maxStackTraces limit', () => { + let iastContext, vulnerability, callSiteFrames + + beforeEach(() => { + iastContext = { + rootSpan: { + meta_struct: { + '_dd.stack': {} + } + } + } + vulnerability = vulnerabilityAnalyzer._createVulnerability( + 'INSECURE_HASHING', + { value: 'sha1' }, + 888, + { path: 'test.js', line: 1 } + ) + callSiteFrames = [{ + getFileName: () => 'test.js', + getLineNumber: () => 1 + }] + }) + + afterEach(() => { + stop() + }) + + it('should report stack trace when under maxStackTraces limit', () => { + start({ + iast: { + deduplicationEnabled: true, + stackTrace: { + enabled: true + } + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } + } + }) + addVulnerability(iastContext, vulnerability, callSiteFrames) + + expect(iastContext.rootSpan.meta_struct['_dd.stack'].vulnerability).to.have.length(1) + }) + + it('should not report stack trace when at maxStackTraces limit', () => { + start({ + iast: { + deduplicationEnabled: true, + stackTrace: { + enabled: true + } + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 1, + maxDepth: 42 + } + } + }) + iastContext.rootSpan.meta_struct['_dd.stack'].vulnerability = ['existing_stack'] + + addVulnerability(iastContext, vulnerability, callSiteFrames) + + expect(iastContext.rootSpan.meta_struct['_dd.stack'].vulnerability).to.have.length(1) + expect(iastContext.rootSpan.meta_struct['_dd.stack'].vulnerability[0]).to.equal('existing_stack') + }) + + it('should always report stack trace when maxStackTraces is 0', () => { + start({ + iast: { + deduplicationEnabled: true, + stackTrace: { + enabled: true + } + }, + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 0, + maxDepth: 42 + } + } + }) + iastContext.rootSpan.meta_struct['_dd.stack'].vulnerability = ['stack1', 'stack2'] + + addVulnerability(iastContext, vulnerability, callSiteFrames) + + expect(iastContext.rootSpan.meta_struct['_dd.stack'].vulnerability).to.have.length(3) + }) + }) + describe('sendVulnerabilities', () => { let span let context @@ -207,7 +303,7 @@ describe('vulnerability-reporter', () => { it('should send one with one vulnerability', () => { const iastContext = { rootSpan: span } addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888), []) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' + @@ -219,7 +315,7 @@ describe('vulnerability-reporter', () => { it('should send only valid vulnerabilities', () => { const iastContext = { rootSpan: span } addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888), []) iastContext.vulnerabilities.push({ invalid: 'vulnerability' }) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ @@ -247,7 +343,8 @@ describe('vulnerability-reporter', () => { } addVulnerability( iastContext, - vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence1, 888, { path: 'filename.js', line: 88 }) + vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence1, 888, { path: 'filename.js', line: 88 }), + [] ) const evidence2 = { @@ -266,7 +363,8 @@ describe('vulnerability-reporter', () => { } addVulnerability( iastContext, - vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence2, 888, { path: 'filename.js', line: 99 }) + vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence2, 888, { path: 'filename.js', line: 99 }), + [] ) sendVulnerabilities(iastContext.vulnerabilities, span) @@ -306,7 +404,8 @@ describe('vulnerability-reporter', () => { } addVulnerability( iastContext, - vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence1, 888, { path: 'filename.js', line: 88 }) + vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence1, 888, { path: 'filename.js', line: 88 }), + [] ) const evidence2 = { @@ -325,7 +424,8 @@ describe('vulnerability-reporter', () => { } addVulnerability( iastContext, - vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence2, 888, { path: 'filename.js', line: 99 }) + vulnerabilityAnalyzer._createVulnerability('SQL_INJECTION', evidence2, 888, { path: 'filename.js', line: 99 }), + [] ) sendVulnerabilities(iastContext.vulnerabilities, span) @@ -349,11 +449,11 @@ describe('vulnerability-reporter', () => { it('should send once with multiple vulnerabilities', () => { const iastContext = { rootSpan: span } addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, - 888, { path: '/path/to/file1.js', line: 1 })) + 888, { path: '/path/to/file1.js', line: 1 }), []) addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'md5' }, 1, - { path: '/path/to/file2.js', line: 1 })) + { path: '/path/to/file2.js', line: 1 }), []) addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'md5' }, -5, - { path: '/path/to/file3.js', line: 3 })) + { path: '/path/to/file3.js', line: 3 }), []) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ '_dd.iast.json': '{"sources":[],"vulnerabilities":[' + @@ -377,7 +477,7 @@ describe('vulnerability-reporter', () => { const iastContext = { rootSpan: span } addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888, - { path: 'filename.js', line: 88 })) + { path: 'filename.js', line: 88 }), []) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + @@ -390,10 +490,10 @@ describe('vulnerability-reporter', () => { const iastContext = { rootSpan: span } addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888, - { path: 'filename.js', line: 88 })) + { path: 'filename.js', line: 88 }), []) addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888, - { path: 'filename.js', line: 88 })) + { path: 'filename.js', line: 88 }), []) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + @@ -420,9 +520,9 @@ describe('vulnerability-reporter', () => { }) const iastContext = { rootSpan: span } addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', - { value: 'sha1' }, 888, { path: 'filename.js', line: 88 })) + { value: 'sha1' }, 888, { path: 'filename.js', line: 88 }), []) addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', - { value: 'sha1' }, 888, { path: 'filename.js', line: 88 })) + { value: 'sha1' }, 888, { path: 'filename.js', line: 88 }), []) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + @@ -441,7 +541,7 @@ describe('vulnerability-reporter', () => { appsecStandalone.configure({ appsec: { standalone: { enabled: true } } }) const iastContext = { rootSpan: span } addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 999)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 999), []) sendVulnerabilities(iastContext.vulnerabilities, span) @@ -459,7 +559,7 @@ describe('vulnerability-reporter', () => { appsecStandalone.configure({ appsec: {} }) const iastContext = { rootSpan: span } addVulnerability(iastContext, - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 999)) + vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 999), []) sendVulnerabilities(iastContext.vulnerabilities, span) @@ -474,87 +574,6 @@ describe('vulnerability-reporter', () => { }) }) - describe('canAddVulnerability', () => { - let vulnerability - - beforeEach(() => { - vulnerability = - vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, undefined, - { path: 'filename.js', line: 73 }, 1) - - clearCache() - }) - - it('should return false when vulnerability is not defined', () => { - expect(canAddVulnerability(null)).to.be.false - }) - - it('should return false when vulnerability is missing required fields', () => { - const incompleteVulnerabilities = [ - { evidence: 'test', type: 'SQL_INJECTION' }, - { evidence: 'test', location: { line: 1 } }, - { type: 'SQL_INJECTION', location: { line: 1 } }, - {} - ] - - incompleteVulnerabilities.forEach(vulnerability => { - expect(canAddVulnerability(vulnerability)).to.be.false - }) - }) - - it('should return true for valid vulnerability when not duplicated', () => { - expect(canAddVulnerability(vulnerability)).to.be.true - }) - - it('should return false for duplicated vulnerability when deduplication is enabled', () => { - start({ - iast: { - deduplicationEnabled: true, - stackTrace: { - enabled: true - } - }, - appsec: { - stackTrace: { - enabled: true, - maxStackTraces: 2, - maxDepth: 42 - } - } - }) - - addVulnerability({ rootSpan: true }, vulnerability) - - expect(canAddVulnerability(vulnerability)).to.be.false - }) - - it('should return true for duplicated vulnerability when deduplication is disabled', () => { - start({ - iast: { - deduplicationEnabled: false, - stackTrace: { - enabled: true - } - }, - appsec: { - stackTrace: { - enabled: true, - maxStackTraces: 2, - maxDepth: 42 - } - } - }) - - addVulnerability({ rootSpan: true }, vulnerability) - - expect(canAddVulnerability(vulnerability)).to.be.true - }) - - afterEach(() => { - clearCache() - }) - }) - describe('clearCache', () => { let span let originalSetInterval @@ -589,17 +608,17 @@ describe('vulnerability-reporter', () => { const vulnerabilityToRepeatInTheNext = vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888, { path: 'filename.js', line: 0 }, 1) - addVulnerability(iastContext, vulnerabilityToRepeatInTheNext) + addVulnerability(iastContext, vulnerabilityToRepeatInTheNext, []) for (let i = 1; i <= MAX; i++) { addVulnerability(iastContext, vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888, - { path: 'filename.js', line: i })) + { path: 'filename.js', line: i }), []) } sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnce const nextIastContext = { rootSpan: span } - addVulnerability(nextIastContext, vulnerabilityToRepeatInTheNext) + addVulnerability(nextIastContext, vulnerabilityToRepeatInTheNext, []) sendVulnerabilities(nextIastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledTwice }) From a17f4d904d6f91f2d2ae141eb44e3c3b80bb32d4 Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 23 Jan 2025 14:31:49 +0100 Subject: [PATCH 14/14] add use strict to utils test file --- packages/dd-trace/test/appsec/utils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/dd-trace/test/appsec/utils.js b/packages/dd-trace/test/appsec/utils.js index b881616b4cf..ec9f22ad283 100644 --- a/packages/dd-trace/test/appsec/utils.js +++ b/packages/dd-trace/test/appsec/utils.js @@ -1,3 +1,5 @@ +'use strict' + function getWebSpan (traces) { for (const trace of traces) { for (const span of trace) {