From b0533223fd8e3d84be3a116de89afb610f90b8da Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 7 Jul 2025 15:35:11 +0200 Subject: [PATCH 1/3] support fastify iast --- .../src/http/server.js | 8 + .../appsec/iast/analyzers/cookie-analyzer.js | 7 +- .../iast/analyzers/vulnerability-analyzer.js | 7 + .../src/appsec/iast/taint-tracking/plugin.js | 101 +++++++--- .../plugin.fastify.plugin.spec.js | 178 ++++++++++++++++++ .../appsec/iast/taint-tracking/plugin.spec.js | 32 ++-- packages/dd-trace/test/appsec/iast/utils.js | 119 +++++++++++- .../test/appsec/index.fastify.plugin.spec.js | 19 +- 8 files changed, 430 insertions(+), 41 deletions(-) create mode 100644 packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js diff --git a/packages/datadog-instrumentations/src/http/server.js b/packages/datadog-instrumentations/src/http/server.js index 0624c886787..5936c8f6c66 100644 --- a/packages/datadog-instrumentations/src/http/server.js +++ b/packages/datadog-instrumentations/src/http/server.js @@ -112,6 +112,14 @@ function wrapWriteHead (writeHead) { // this doesn't support explicit duplicate headers, but it's an edge case const responseHeaders = Object.assign(this.getHeaders(), obj) + // Publish all headers from writeHead to ensure frameworks like Fastify + // that bypass setHeader() are still captured for analysis + if (finishSetHeaderCh.hasSubscribers) { + for (const [name, value] of Object.entries(responseHeaders)) { + finishSetHeaderCh.publish({ name, value, res: this }) + } + } + startWriteHeadCh.publish({ req: this.req, res: this, 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 f422e5b363c..a63bf8b36d8 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js @@ -3,7 +3,12 @@ const Analyzer = require('./vulnerability-analyzer') const { getNodeModulesPaths } = require('../path-line') -const EXCLUDED_PATHS = getNodeModulesPaths('express/lib/response.js') +const EXCLUDED_PATHS = [ + getNodeModulesPaths('express/lib/response.js'), + getNodeModulesPaths('fastify/lib/reply.js'), + getNodeModulesPaths('fastify/lib/hooks.js'), + getNodeModulesPaths('@fastify/cookie/plugin.js'), +] class CookieAnalyzer extends Analyzer { constructor (type, propertyToBeSafe) { 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 c328b520362..34bacd99fb3 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -39,6 +39,13 @@ class Analyzer extends SinkIastPlugin { const location = this._getLocation(value, nonDDCallSiteFrames) + if (this._type === "INSECURE_COOKIE") { + console.log('======>>> callSiteFrames ', callSiteFrames) + console.log('======>>> nonDDCallSiteFrames ', nonDDCallSiteFrames) + console.log('======>>> location ', location) + } + + if (!this._isExcluded(location)) { const originalLocation = this._getOriginalLocation(location) const spanId = context?.rootSpan?.context().toSpanId() diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js b/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js index c106ec9a6a3..bd910c25cf4 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js @@ -38,6 +38,25 @@ class TaintTrackingPlugin extends SourceIastPlugin { } onConfigure () { + this.addBodyParsingSubscriptions() + + this.addQueryParameterSubscriptions() + + this.addCookieSubscriptions() + + this.addDatabaseSubscriptions() + + this.addPathParameterSubscriptions() + + this.addGraphQLSubscriptions() + + this.addURLParsingSubscriptions() + + // this is a special case to increment INSTRUMENTED_SOURCE metric for header + this.addInstrumentedSource('http', [HTTP_REQUEST_HEADER_VALUE, HTTP_REQUEST_HEADER_NAME]) + } + + addBodyParsingSubscriptions () { const onRequestBody = ({ req }) => { const iastContext = getIastContext(storage('legacy').getStore()) if (iastContext && iastContext.body !== req.body) { @@ -57,17 +76,14 @@ class TaintTrackingPlugin extends SourceIastPlugin { ) this.addSub( - { channelName: 'datadog:query:read:finish', tag: HTTP_REQUEST_PARAMETER }, - ({ query }) => this._taintTrackingHandler(HTTP_REQUEST_PARAMETER, query) - ) - - this.addSub( - { channelName: 'datadog:express:query:finish', tag: HTTP_REQUEST_PARAMETER }, - ({ query }) => { + { channelName: 'datadog:fastify:body-parser:finish', tag: HTTP_REQUEST_BODY }, + ({ req, body }) => { const iastContext = getIastContext(storage('legacy').getStore()) - if (!iastContext || !query) return - - taintQueryWithCache(iastContext, query) + if (iastContext && iastContext.body !== body) { + req.body = body + this._taintTrackingHandler(HTTP_REQUEST_BODY, req, 'body', iastContext) + iastContext.body = body + } } ) @@ -83,12 +99,45 @@ class TaintTrackingPlugin extends SourceIastPlugin { } } ) + } + addQueryParameterSubscriptions () { + this.addSub( + { channelName: 'datadog:query:read:finish', tag: HTTP_REQUEST_PARAMETER }, + ({ query }) => this._taintTrackingHandler(HTTP_REQUEST_PARAMETER, query) + ) + + this.addSub( + { channelName: 'datadog:fastify:query-params:finish', tag: HTTP_REQUEST_PARAMETER }, + ({ query }) => { + this._taintTrackingHandler(HTTP_REQUEST_PARAMETER, query) + } + ) + + this.addSub( + { channelName: 'datadog:express:query:finish', tag: HTTP_REQUEST_PARAMETER }, + ({ query }) => { + const iastContext = getIastContext(storage('legacy').getStore()) + if (!iastContext || !query) return + + taintQueryWithCache(iastContext, query) + } + ) + } + + addCookieSubscriptions () { this.addSub( { channelName: 'datadog:cookie:parse:finish', tag: [HTTP_REQUEST_COOKIE_VALUE, HTTP_REQUEST_COOKIE_NAME] }, ({ cookies }) => this._cookiesTaintTrackingHandler(cookies) ) + this.addSub( + { channelName: 'datadog:fastify-cookie:read:finish', tag: [HTTP_REQUEST_COOKIE_VALUE, HTTP_REQUEST_COOKIE_NAME] }, + ({ cookies }) => this._cookiesTaintTrackingHandler(cookies) + ) + } + + addDatabaseSubscriptions () { this.addSub( { channelName: 'datadog:sequelize:query:finish', tag: SQL_ROW_VALUE }, ({ result }) => this._taintDatabaseResult(result, 'sequelize') @@ -98,25 +147,32 @@ class TaintTrackingPlugin extends SourceIastPlugin { { channelName: 'apm:pg:query:finish', tag: SQL_ROW_VALUE }, ({ result }) => this._taintDatabaseResult(result, 'pg') ) + } + + addPathParameterSubscriptions () { + const pathParamHandler = ({ req }) => { + if (req && req.params !== null && typeof req.params === 'object') { + this._taintTrackingHandler(HTTP_REQUEST_PATH_PARAM, req, 'params') + } + } this.addSub( { channelName: 'datadog:express:process_params:start', tag: HTTP_REQUEST_PATH_PARAM }, - ({ req }) => { - if (req && req.params !== null && typeof req.params === 'object') { - this._taintTrackingHandler(HTTP_REQUEST_PATH_PARAM, req, 'params') - } - } + pathParamHandler + ) + + this.addSub( + { channelName: 'datadog:fastify:path-params:finish', tag: HTTP_REQUEST_PATH_PARAM }, + pathParamHandler ) this.addSub( { channelName: 'datadog:router:param:start', tag: HTTP_REQUEST_PATH_PARAM }, - ({ req }) => { - if (req && req.params !== null && typeof req.params === 'object') { - this._taintTrackingHandler(HTTP_REQUEST_PATH_PARAM, req, 'params') - } - } + pathParamHandler ) + } + addGraphQLSubscriptions () { this.addSub( { channelName: 'apm:graphql:resolve:start', tag: HTTP_REQUEST_BODY }, (data) => { @@ -128,7 +184,9 @@ class TaintTrackingPlugin extends SourceIastPlugin { } } ) + } + addURLParsingSubscriptions () { const urlResultTaintedProperties = ['host', 'origin', 'hostname'] this.addSub( { channelName: 'datadog:url:parse:finish' }, @@ -162,9 +220,6 @@ class TaintTrackingPlugin extends SourceIastPlugin { context.result = newTaintedString(iastContext, context.result, origRange.iinfo.parameterName, origRange.iinfo.type) }) - - // this is a special case to increment INSTRUMENTED_SOURCE metric for header - this.addInstrumentedSource('http', [HTTP_REQUEST_HEADER_VALUE, HTTP_REQUEST_HEADER_NAME]) } _taintTrackingHandler (type, target, property, iastContext = getIastContext(storage('legacy').getStore())) { diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js new file mode 100644 index 00000000000..be85705d6d3 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js @@ -0,0 +1,178 @@ +'use strict' + +const { prepareTestServerForIastInFastify } = require('../utils') +const axios = require('axios') +const { URL } = require('url') + +function noop () {} + +describe('Taint tracking plugin sources fastify tests', () => { + withVersions('fastify', 'fastify', '>=2', version => { + prepareTestServerForIastInFastify('in fastify', version, + (testThatRequestHasVulnerability, _, config) => { + describe.skip('tainted body', () => { + function makePostRequest (done) { + axios.post(`http://localhost:${config.port}/`, { + command: 'echo 1' + }).catch(done) + } + + testThatRequestHasVulnerability((req) => { + const childProcess = require('child_process') + childProcess.exec(req.body.command, noop) + }, 'COMMAND_INJECTION', 1, noop, makePostRequest) + }) + + describe.skip('tainted query param', () => { + function makeRequestWithQueryParam (done) { + axios.get(`http://localhost:${config.port}/?command=echo`).catch(done) + } + + testThatRequestHasVulnerability((req) => { + const childProcess = require('child_process') + childProcess.exec(req.query.command, noop) + }, 'COMMAND_INJECTION', 1, noop, makeRequestWithQueryParam) + }) + + describe.skip('tainted header', () => { + function makeRequestWithHeader (done) { + axios.get(`http://localhost:${config.port}/`, { + headers: { + 'x-iast-test-command': 'echo 1' + } + }).catch(done) + } + + testThatRequestHasVulnerability((req) => { + const childProcess = require('child_process') + childProcess.exec(req.headers['x-iast-test-command'], noop) + }, 'COMMAND_INJECTION', 1, noop, makeRequestWithHeader) + }) + + describe.skip('url parse taint tracking', () => { + function makePostRequest (done) { + axios.post(`http://localhost:${config.port}/`, { + url: 'http://www.datadoghq.com/' + }).catch(done) + } + + testThatRequestHasVulnerability( + { + fn: (req) => { + // eslint-disable-next-line n/no-deprecated-api + const { parse } = require('url') + const url = parse(req.body.url) + + const childProcess = require('child_process') + childProcess.exec(url.host, noop) + }, + vulnerability: 'COMMAND_INJECTION', + occurrences: 1, + cb: noop, + makeRequest: makePostRequest, + testDescription: 'should detect vulnerability when tainted is coming from url.parse' + }) + + testThatRequestHasVulnerability( + { + fn: (req) => { + const { URL } = require('url') + const url = new URL(req.body.url) + + const childProcess = require('child_process') + childProcess.exec(url.host, noop) + }, + vulnerability: 'COMMAND_INJECTION', + occurrences: 1, + cb: noop, + makeRequest: makePostRequest, + testDescription: 'should detect vulnerability when tainted is coming from new url.URL input' + }) + + testThatRequestHasVulnerability( + { + fn: (req) => { + const { URL } = require('url') + const url = new URL('/path', req.body.url) + + const childProcess = require('child_process') + childProcess.exec(url.host, noop) + }, + vulnerability: 'COMMAND_INJECTION', + occurrences: 1, + cb: noop, + makeRequest: makePostRequest, + testDescription: 'should detect vulnerability when tainted is coming from new url.URL base' + }) + + if (URL.parse) { + testThatRequestHasVulnerability( + { + fn: (req) => { + const { URL } = require('url') + const url = URL.parse(req.body.url) + const childProcess = require('child_process') + childProcess.exec(url.host, noop) + }, + vulnerability: 'COMMAND_INJECTION', + occurrences: 1, + cb: noop, + makeRequest: makePostRequest, + testDescription: 'should detect vulnerability when tainted is coming from url.URL.parse input' + }) + + testThatRequestHasVulnerability( + { + fn: (req) => { + const { URL } = require('url') + const url = URL.parse('/path', req.body.url) + const childProcess = require('child_process') + childProcess.exec(url.host, noop) + }, + vulnerability: 'COMMAND_INJECTION', + occurrences: 1, + cb: noop, + makeRequest: makePostRequest, + testDescription: 'should detect vulnerability when tainted is coming from url.URL.parse base' + }) + } + }) + + describe.skip('tainted path parameters', () => { + function makeRequestWithPathParam (done) { + // Note: This would require setting up a parameterized route in Fastify + // For now, using query params as a substitute since the test setup is generic + axios.get(`http://localhost:${config.port}/?id=malicious-path`).catch(done) + } + + testThatRequestHasVulnerability((req) => { + const childProcess = require('child_process') + // Simulating path parameter access through query for test purposes + childProcess.exec(req.query.id, noop) + }, 'COMMAND_INJECTION', 1, noop, makeRequestWithPathParam) + }) + + // NO + // describe.skip('tainted cookies', () => { + // function makeRequestWithCookie (done) { + // axios.get(`http://localhost:${config.port}/`, { + // headers: { + // Cookie: 'command=echo cookie-injection' + // } + // }).catch(done) + // } + + // testThatRequestHasVulnerability((req) => { + // const childProcess = require('child_process') + // // Access cookie through raw request since Fastify cookie parsing may vary + // const cookieHeader = req.headers.cookie + // if (cookieHeader) { + // const command = cookieHeader.split('=')[1] + // childProcess.exec(command, noop) + // } + // }, 'COMMAND_INJECTION', 1, noop, makeRequestWithCookie) + // }) + } + ) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js index 5d15dafdf28..0fa4e46959d 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js @@ -49,20 +49,24 @@ describe('IAST Taint tracking plugin', () => { }) it('Should subscribe to body parser, qs, cookie and process_params channel', () => { - expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(13) - expect(taintTrackingPlugin._subscriptions[0]._channel.name).to.equals('datadog:body-parser:read:finish') - expect(taintTrackingPlugin._subscriptions[1]._channel.name).to.equals('datadog:multer:read:finish') - expect(taintTrackingPlugin._subscriptions[2]._channel.name).to.equals('datadog:query:read:finish') - expect(taintTrackingPlugin._subscriptions[3]._channel.name).to.equals('datadog:express:query:finish') - expect(taintTrackingPlugin._subscriptions[4]._channel.name).to.equals('apm:express:middleware:next') - expect(taintTrackingPlugin._subscriptions[5]._channel.name).to.equals('datadog:cookie:parse:finish') - expect(taintTrackingPlugin._subscriptions[6]._channel.name).to.equals('datadog:sequelize:query:finish') - expect(taintTrackingPlugin._subscriptions[7]._channel.name).to.equals('apm:pg:query:finish') - expect(taintTrackingPlugin._subscriptions[8]._channel.name).to.equals('datadog:express:process_params:start') - expect(taintTrackingPlugin._subscriptions[9]._channel.name).to.equals('datadog:router:param:start') - expect(taintTrackingPlugin._subscriptions[10]._channel.name).to.equals('apm:graphql:resolve:start') - expect(taintTrackingPlugin._subscriptions[11]._channel.name).to.equals('datadog:url:parse:finish') - expect(taintTrackingPlugin._subscriptions[12]._channel.name).to.equals('datadog:url:getter:finish') + expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(16) + let i = 0 + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:body-parser:read:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:multer:read:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify:body-parser:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('apm:express:middleware:next') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:query:read:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify:query-params:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:express:query:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:cookie:parse:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:sequelize:query:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('apm:pg:query:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:express:process_params:start') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify:path-params:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:router:param:start') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('apm:graphql:resolve:start') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:url:parse:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:url:getter:finish') }) describe('taint sources', () => { diff --git a/packages/dd-trace/test/appsec/iast/utils.js b/packages/dd-trace/test/appsec/iast/utils.js index 932f23e5d3c..3f81a6d3c51 100644 --- a/packages/dd-trace/test/appsec/iast/utils.js +++ b/packages/dd-trace/test/appsec/iast/utils.js @@ -410,6 +410,122 @@ function prepareTestServerForIastInExpress (description, expressVersion, loadMid }) } +function prepareTestServerForIastInFastify (description, fastifyVersion, tests, iastConfig) { + describe(description, () => { + const config = {} + let app, server + + before(function () { + if (fastifyVersion === '3.9.2') { + this.skip() + } + + return agent.load(['fastify', 'http'], { client: false }, { flushInterval: 1 }) + }) + + before(async () => { + const fastify = require(`../../../../../versions/fastify@${fastifyVersion}`).get() + const fastifyCookie = require('../../../../../versions/@fastify/cookie').get() + + const fastifyApp = fastify() + + fastifyApp.register(fastifyCookie) + + fastifyApp.all('/', (request, reply) => { + const headersSent = () => { + if (reply.raw && typeof reply.raw.headersSent !== 'undefined') { + return reply.raw.headersSent + } + // Fastify <3: use reply.sent indicator + return reply.sent === true + } + + try { + const result = app && app(request, reply.raw) + + const finish = () => { + if (!headersSent()) { + reply.code(200).send() + } + } + + if (result && typeof result.then === 'function') { + result.then(finish).catch(() => finish()) + } else { + finish() + } + } catch (e) { + if (!headersSent()) { + reply.code(500).send() + } else if (reply.raw && typeof reply.raw.end === 'function') { + reply.raw.end() + } + } + }) + + await fastifyApp.listen({ port: 0 }) + + server = fastifyApp.server + config.port = server.address().port + }) + + beforeEachIastTest(iastConfig) + + afterEach(() => { + iast.disable() + rewriter.disable() + app = null + }) + + after(() => { + server?.close() + return agent?.close({ ritmReset: false }) + }) + + function testThatRequestHasVulnerability (fn, vulnerability, occurrencesAndLocation, cb, makeRequest) { + let testDescription + if (fn !== null && typeof fn === 'object') { + const obj = fn + fn = obj.fn + vulnerability = obj.vulnerability + occurrencesAndLocation = obj.occurrencesAndLocation || obj.occurrences + cb = obj.cb + makeRequest = obj.makeRequest + testDescription = obj.testDescription || testDescription + } + + testDescription = testDescription || `should have ${vulnerability} vulnerability` + + it(testDescription, function (done) { + this.timeout(5000) + app = fn + + checkVulnerabilityInRequest(vulnerability, occurrencesAndLocation, cb, makeRequest, config, done) + }) + } + + function testThatRequestHasNoVulnerability (fn, vulnerability, makeRequest) { + let testDescription + if (fn !== null && typeof fn === 'object') { + const obj = fn + fn = obj.fn + vulnerability = obj.vulnerability + makeRequest = obj.makeRequest + testDescription = obj.testDescription || testDescription + } + + testDescription = testDescription || `should not have ${vulnerability} vulnerability` + + it(testDescription, function (done) { + app = fn + checkNoVulnerabilityInRequest(vulnerability, config, done, makeRequest) + }) + } + + tests(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability, config) + }) +} + function locationHasMatchingFrame (span, vulnerabilityType, vulnerabilities) { const stack = msgpack.decode(span.meta_struct['_dd.stack']) const matchingVulns = vulnerabilities.filter(vulnerability => vulnerability.type === vulnerabilityType) @@ -438,5 +554,6 @@ module.exports = { testInRequest, copyFileToTmp, prepareTestServerForIast, - prepareTestServerForIastInExpress + prepareTestServerForIastInExpress, + prepareTestServerForIastInFastify } diff --git a/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js b/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js index a9d5804c9cc..ee343ebfb8f 100644 --- a/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js +++ b/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js @@ -11,7 +11,7 @@ const appsec = require('../../src/appsec') const Config = require('../../src/config') const { json } = require('../../src/appsec/blocked_templates') -withVersions('fastify', 'fastify', '>=2', version => { +withVersions('fastify', 'fastify', '5.4.0', version => { describe('Suspicious request blocking - query', () => { let server, requestBody, axios @@ -433,7 +433,7 @@ withVersions('fastify', 'fastify', '>=2', version => { }) describe('Suspicious request blocking - cookie', () => { - withVersions('fastify', '@fastify/cookie', cookieVersion => { + withVersions('fastify', '@fastify/cookie', '11.0.2', cookieVersion => { const hookConfigurations = [ 'onRequest', 'preParsing', @@ -478,6 +478,11 @@ withVersions('fastify', 'fastify', '>=2', version => { reply.send('DONE') }) + app.get('/test-cookie', (request, reply) => { + reply.cookie('test', 'value', { httpOnly: true }) + reply.send('Cookie set') + }) + getPort().then((port) => { app.listen({ port }, () => { axios = Axios.create({ baseURL: `http://localhost:${port}` }) @@ -510,6 +515,16 @@ withVersions('fastify', 'fastify', '>=2', version => { return agent.close({ ritmReset: false }) }) + it.only('should trigger HTTP header instrumentation when setting cookies', async () => { + const res = await axios.get('/test-cookie') + + console.log('Response headers:', res.headers) + assert.equal(res.status, 200) + assert.equal(res.data, 'Cookie set') + // Check if Set-Cookie header was set + assert.property(res.headers, 'set-cookie') + }) + it('should not block the request without an attack', async () => { const res = await axios.post('/', {}) From d7bdb687ca6fe9d3006cd0c496cea43064cde556 Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 8 Jul 2025 09:05:33 +0200 Subject: [PATCH 2/3] instrument fastify reply header --- .../datadog-instrumentations/src/fastify.js | 16 ++++++++ .../src/http/server.js | 8 ---- .../analyzers/hsts-header-missing-analyzer.js | 4 +- .../iast/analyzers/missing-header-analyzer.js | 20 +++++----- .../set-cookies-header-interceptor.js | 40 ++++++++++--------- .../unvalidated-redirect-analyzer.js | 6 ++- .../xcontenttype-header-missing-analyzer.js | 4 +- packages/dd-trace/src/appsec/iast/index.js | 23 ++++++++++- .../test/appsec/index.fastify.plugin.spec.js | 2 +- 9 files changed, 80 insertions(+), 43 deletions(-) diff --git a/packages/datadog-instrumentations/src/fastify.js b/packages/datadog-instrumentations/src/fastify.js index 1f355a49a02..48405611f23 100644 --- a/packages/datadog-instrumentations/src/fastify.js +++ b/packages/datadog-instrumentations/src/fastify.js @@ -11,6 +11,7 @@ const queryParamsReadCh = channel('datadog:fastify:query-params:finish') const cookieParserReadCh = channel('datadog:fastify-cookie:read:finish') const responsePayloadReadCh = channel('datadog:fastify:response:finish') const pathParamsReadCh = channel('datadog:fastify:path-params:finish') +const finishSetHeaderCh = channel('datadog:fastify:set-header:finish') const parsingResources = new WeakMap() const cookiesPublished = new WeakSet() @@ -257,3 +258,18 @@ addHook({ name: 'fastify', versions: ['2'] }, fastify => { addHook({ name: 'fastify', versions: ['1'] }, fastify => { return shimmer.wrapFunction(fastify, fastify => wrapFastify(fastify, false)) }) + +addHook({ name: 'fastify', file: 'lib/reply.js', versions: ['>=3'] }, Reply => { + + shimmer.wrap(Reply.prototype, 'header', header => function (key, value = '') { + const result = header.apply(this, arguments) + + if (finishSetHeaderCh.hasSubscribers && key && value) { + finishSetHeaderCh.publish({ name: key, value, res: this.raw }) + } + + return result + }) + + return Reply +}) diff --git a/packages/datadog-instrumentations/src/http/server.js b/packages/datadog-instrumentations/src/http/server.js index 5936c8f6c66..0624c886787 100644 --- a/packages/datadog-instrumentations/src/http/server.js +++ b/packages/datadog-instrumentations/src/http/server.js @@ -112,14 +112,6 @@ function wrapWriteHead (writeHead) { // this doesn't support explicit duplicate headers, but it's an edge case const responseHeaders = Object.assign(this.getHeaders(), obj) - // Publish all headers from writeHead to ensure frameworks like Fastify - // that bypass setHeader() are still captured for analysis - if (finishSetHeaderCh.hasSubscribers) { - for (const [name, value] of Object.entries(responseHeaders)) { - finishSetHeaderCh.publish({ name, value, res: this }) - } - } - startWriteHeadCh.publish({ req: this.req, res: this, diff --git a/packages/dd-trace/src/appsec/iast/analyzers/hsts-header-missing-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/hsts-header-missing-analyzer.js index da1d1a8fa49..fa7efb4f227 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/hsts-header-missing-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/hsts-header-missing-analyzer.js @@ -10,8 +10,8 @@ class HstsHeaderMissingAnalyzer extends MissingHeaderAnalyzer { super(HSTS_HEADER_MISSING, HSTS_HEADER_NAME) } - _isVulnerableFromRequestAndResponse (req, res) { - const headerValues = this._getHeaderValues(res, HSTS_HEADER_NAME) + _isVulnerableFromRequestAndResponse (req, res, storedHeaders) { + const headerValues = this._getHeaderValues(res, storedHeaders, HSTS_HEADER_NAME) return this._isHttpsProtocol(req) && ( headerValues.length === 0 || headerValues.some(headerValue => !this._isHeaderValid(headerValue)) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/missing-header-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/missing-header-analyzer.js index d8a847a4042..5dc228a30af 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/missing-header-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/missing-header-analyzer.js @@ -28,8 +28,8 @@ class MissingHeaderAnalyzer extends Analyzer { }, (data) => this.analyze(data)) } - _getHeaderValues (res, headerName) { - const headerValue = res.getHeader(headerName) + _getHeaderValues (res, storedHeaders, headerName) { + const headerValue = res.getHeader(headerName) || storedHeaders[headerName] if (Array.isArray(headerValue)) { return headerValue } @@ -46,8 +46,8 @@ class MissingHeaderAnalyzer extends Analyzer { return `${type}:${this.config.tracerConfig.service}` } - _getEvidence ({ res }) { - const headerValues = this._getHeaderValues(res, this.headerName) + _getEvidence ({ res, storedHeaders }) { + const headerValues = this._getHeaderValues(res, storedHeaders, this.headerName) let value if (headerValues.length === 1) { value = headerValues[0] @@ -57,19 +57,19 @@ class MissingHeaderAnalyzer extends Analyzer { return { value } } - _isVulnerable ({ req, res }, context) { - if (!IGNORED_RESPONSE_STATUS_LIST.has(res.statusCode) && this._isResponseHtml(res)) { - return this._isVulnerableFromRequestAndResponse(req, res) + _isVulnerable ({ req, res, storedHeaders }, context) { + if (!IGNORED_RESPONSE_STATUS_LIST.has(res.statusCode) && this._isResponseHtml(res, storedHeaders)) { + return this._isVulnerableFromRequestAndResponse(req, res, storedHeaders) } return false } - _isVulnerableFromRequestAndResponse (req, res) { + _isVulnerableFromRequestAndResponse (req, res, storedHeaders) { return false } - _isResponseHtml (res) { - const contentTypes = this._getHeaderValues(res, 'content-type') + _isResponseHtml (res, storedHeaders) { + const contentTypes = this._getHeaderValues(res, storedHeaders, 'content-type') return contentTypes.some(contentType => { return contentType && HTML_CONTENT_TYPES.some(htmlContentType => { return htmlContentType === contentType || contentType.startsWith(htmlContentType + ';') diff --git a/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js b/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js index c7de02c6e53..711a02d2139 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js @@ -7,25 +7,29 @@ class SetCookiesHeaderInterceptor extends Plugin { constructor () { super() this.cookiesInRequest = new WeakMap() - this.addSub('datadog:http:server:response:set-header:finish', ({ name, value, res }) => { - if (name.toLowerCase() === 'set-cookie') { - let allCookies = value - if (typeof value === 'string') { - allCookies = [value] - } - const alreadyCheckedCookies = this._getAlreadyCheckedCookiesInResponse(res) - - let location - allCookies.forEach(cookieString => { - if (!alreadyCheckedCookies.includes(cookieString)) { - alreadyCheckedCookies.push(cookieString) - const parsedCookie = this._parseCookie(cookieString, location) - setCookieChannel.publish(parsedCookie) - location = parsedCookie.location - } - }) + + this.addSub('datadog:http:server:response:set-header:finish', ({ name, value, res }) => this._handleCookies(name, value, res)) + this.addSub('datadog:fastify:set-header:finish', ({ name, value, res }) => this._handleCookies(name, value, res)) + } + + _handleCookies (name, value, res) { + if (name.toLowerCase() === 'set-cookie') { + let allCookies = value + if (typeof value === 'string') { + allCookies = [value] } - }) + const alreadyCheckedCookies = this._getAlreadyCheckedCookiesInResponse(res) + + let location + allCookies.forEach(cookieString => { + if (!alreadyCheckedCookies.includes(cookieString)) { + alreadyCheckedCookies.push(cookieString) + const parsedCookie = this._parseCookie(cookieString, location) + setCookieChannel.publish(parsedCookie) + location = parsedCookie.location + } + }) + } } _parseCookie (cookieString, location) { diff --git a/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js index c2e485c1c9a..67057dddeef 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/unvalidated-redirect-analyzer.js @@ -9,7 +9,10 @@ const { HTTP_REQUEST_PARAMETER } = require('../taint-tracking/source-types') -const EXCLUDED_PATHS = getNodeModulesPaths('express/lib/response.js') +const EXCLUDED_PATHS = [ + getNodeModulesPaths('express/lib/response.js'), + getNodeModulesPaths('fastify/lib/reply.js'), +] const VULNERABLE_SOURCE_TYPES = new Set([ HTTP_REQUEST_BODY, @@ -23,6 +26,7 @@ class UnvalidatedRedirectAnalyzer extends InjectionAnalyzer { onConfigure () { this.addSub('datadog:http:server:response:set-header:finish', ({ name, value }) => this.analyze(name, value)) + this.addSub('datadog:fastify:set-header:finish', ({ name, value }) => this.analyze(name, value)) } analyze (name, value) { diff --git a/packages/dd-trace/src/appsec/iast/analyzers/xcontenttype-header-missing-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/xcontenttype-header-missing-analyzer.js index 6d114d0b168..665249bfabb 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/xcontenttype-header-missing-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/xcontenttype-header-missing-analyzer.js @@ -10,8 +10,8 @@ class XcontenttypeHeaderMissingAnalyzer extends MissingHeaderAnalyzer { super(XCONTENTTYPE_HEADER_MISSING, XCONTENTTYPEOPTIONS_HEADER_NAME) } - _isVulnerableFromRequestAndResponse (req, res) { - const headerValues = this._getHeaderValues(res, XCONTENTTYPEOPTIONS_HEADER_NAME) + _isVulnerableFromRequestAndResponse (req, res, storedHeaders) { + const headerValues = this._getHeaderValues(res, storedHeaders, XCONTENTTYPEOPTIONS_HEADER_NAME) return headerValues.length === 0 || headerValues.some(headerValue => headerValue.trim().toLowerCase() !== 'nosniff') } } diff --git a/packages/dd-trace/src/appsec/iast/index.js b/packages/dd-trace/src/appsec/iast/index.js index 8601dc3c8ca..87c6f822c77 100644 --- a/packages/dd-trace/src/appsec/iast/index.js +++ b/packages/dd-trace/src/appsec/iast/index.js @@ -16,6 +16,10 @@ const { IAST_ENABLED_TAG_KEY } = require('./tags') const iastTelemetry = require('./telemetry') const { enable: enableFsPlugin, disable: disableFsPlugin, IAST_MODULE } = require('../rasp/fs-plugin') const securityControls = require('./security-controls') +const { responseWriteHead } = require('../channels') + +// Map raw response -> accumulated headers set via writeHead +const collectedResponseHeaders = new WeakMap() // TODO Change to `apm:http:server:request:[start|close]` when the subscription // order of the callbacks can be enforce @@ -33,6 +37,7 @@ function enable (config, _tracer) { enableTaintTracking(config.iast, iastTelemetry.verbosity) requestStart.subscribe(onIncomingHttpRequestStart) requestClose.subscribe(onIncomingHttpRequestEnd) + responseWriteHead.subscribe(onResponseWriteHeadCollect) overheadController.configure(config.iast) overheadController.startGlobalContext() securityControls.configure(config.iast) @@ -53,6 +58,7 @@ function disable () { overheadController.finishGlobalContext() if (requestStart.hasSubscribers) requestStart.unsubscribe(onIncomingHttpRequestStart) if (requestClose.hasSubscribers) requestClose.unsubscribe(onIncomingHttpRequestEnd) + if (responseWriteHead.hasSubscribers) responseWriteHead.unsubscribe(onResponseWriteHeadCollect) vulnerabilityReporter.stop() } @@ -87,7 +93,13 @@ function onIncomingHttpRequestEnd (data) { const topContext = web.getContext(data.req) const iastContext = iastContextFunctions.getIastContext(store, topContext) if (iastContext?.rootSpan) { - iastResponseEnd.publish(data) + const storedHeaders = collectedResponseHeaders.get(data.res) || {} + + iastResponseEnd.publish({ ...data, storedHeaders }) + + if (storedHeaders) { + collectedResponseHeaders.delete(data.res) + } const vulnerabilities = iastContext.vulnerabilities const rootSpan = iastContext.rootSpan @@ -103,4 +115,13 @@ function onIncomingHttpRequestEnd (data) { } } +function onResponseWriteHeadCollect ({ res, responseHeaders = {} }) { + if (!res) return + + if (Object.keys(responseHeaders).length) { + collectedResponseHeaders.set(res, responseHeaders) + } +} + + module.exports = { enable, disable, onIncomingHttpRequestEnd, onIncomingHttpRequestStart } diff --git a/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js b/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js index ee343ebfb8f..a0fab962236 100644 --- a/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js +++ b/packages/dd-trace/test/appsec/index.fastify.plugin.spec.js @@ -515,7 +515,7 @@ withVersions('fastify', 'fastify', '5.4.0', version => { return agent.close({ ritmReset: false }) }) - it.only('should trigger HTTP header instrumentation when setting cookies', async () => { + it('should trigger HTTP header instrumentation when setting cookies', async () => { const res = await axios.get('/test-cookie') console.log('Response headers:', res.headers) From c25c8dc84e82243dc0b55a905bc554a8df1cde5f Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 10 Jul 2025 14:28:54 +0200 Subject: [PATCH 3/3] linter --- packages/datadog-instrumentations/src/fastify.js | 1 - .../iast/analyzers/set-cookies-header-interceptor.js | 7 +++++-- packages/dd-trace/src/appsec/iast/index.js | 1 - .../iast/analyzers/unvalidated-redirect-analyzer.spec.js | 4 +++- .../iast/taint-tracking/plugin.fastify.plugin.spec.js | 2 +- .../test/appsec/iast/taint-tracking/plugin.spec.js | 5 +++-- .../sources/taint-tracking.fastify.plugin.spec.js | 3 +-- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/datadog-instrumentations/src/fastify.js b/packages/datadog-instrumentations/src/fastify.js index 6bdec7153ee..25ce61fa4da 100644 --- a/packages/datadog-instrumentations/src/fastify.js +++ b/packages/datadog-instrumentations/src/fastify.js @@ -259,7 +259,6 @@ addHook({ name: 'fastify', versions: ['1'] }, fastify => { return shimmer.wrapFunction(fastify, fastify => wrapFastify(fastify, false)) }) - function wrapReplyHeader (Reply) { shimmer.wrap(Reply.prototype, 'header', header => function (key, value) { const result = header.apply(this, arguments) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js b/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js index 711a02d2139..a880a4cd84f 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/set-cookies-header-interceptor.js @@ -8,8 +8,11 @@ class SetCookiesHeaderInterceptor extends Plugin { super() this.cookiesInRequest = new WeakMap() - this.addSub('datadog:http:server:response:set-header:finish', ({ name, value, res }) => this._handleCookies(name, value, res)) - this.addSub('datadog:fastify:set-header:finish', ({ name, value, res }) => this._handleCookies(name, value, res)) + this.addSub('datadog:http:server:response:set-header:finish', + ({ name, value, res }) => this._handleCookies(name, value, res)) + + this.addSub('datadog:fastify:set-header:finish', + ({ name, value, res }) => this._handleCookies(name, value, res)) } _handleCookies (name, value, res) { diff --git a/packages/dd-trace/src/appsec/iast/index.js b/packages/dd-trace/src/appsec/iast/index.js index 2cef1756e43..6c80f19be6b 100644 --- a/packages/dd-trace/src/appsec/iast/index.js +++ b/packages/dd-trace/src/appsec/iast/index.js @@ -123,5 +123,4 @@ function onResponseWriteHeadCollect ({ res, responseHeaders = {} }) { } } - module.exports = { enable, disable, onIncomingHttpRequestEnd, onIncomingHttpRequestStart } diff --git a/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js index d038f7d023b..ebab45cbbb6 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/unvalidated-redirect-analyzer.spec.js @@ -97,9 +97,11 @@ describe('unvalidated-redirect-analyzer', () => { unvalidatedRedirectAnalyzer.configure(true) it('should subscribe to set-header:finish channel', () => { - expect(unvalidatedRedirectAnalyzer._subscriptions).to.have.lengthOf(1) + expect(unvalidatedRedirectAnalyzer._subscriptions).to.have.lengthOf(2) expect(unvalidatedRedirectAnalyzer._subscriptions[0]._channel.name).to .equals('datadog:http:server:response:set-header:finish') + expect(unvalidatedRedirectAnalyzer._subscriptions[1]._channel.name).to + .equals('datadog:fastify:set-header:finish') }) it('should not report headers other than Location', () => { diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js index d3538896244..26d0b94bcdf 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.fastify.plugin.spec.js @@ -7,7 +7,7 @@ const { URL } = require('url') function noop () {} describe('Taint tracking plugin sources fastify tests', () => { - withVersions('fastify', 'fastify', version => { + withVersions('fastify', 'fastify', '>=2', version => { prepareTestServerForIastInFastify('in fastify', version, (testThatRequestHasVulnerability, _, config) => { describe('tainted body', () => { diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js index 0fa4e46959d..678c18b4981 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/plugin.spec.js @@ -49,7 +49,7 @@ describe('IAST Taint tracking plugin', () => { }) it('Should subscribe to body parser, qs, cookie and process_params channel', () => { - expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(16) + expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(17) let i = 0 expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:body-parser:read:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:multer:read:finish') @@ -59,11 +59,12 @@ describe('IAST Taint tracking plugin', () => { expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify:query-params:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:express:query:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:cookie:parse:finish') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify-cookie:read:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:sequelize:query:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('apm:pg:query:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:express:process_params:start') - expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify:path-params:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:router:param:start') + expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:fastify:path-params:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('apm:graphql:resolve:start') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:url:parse:finish') expect(taintTrackingPlugin._subscriptions[i++]._channel.name).to.equals('datadog:url:getter:finish') diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.fastify.plugin.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.fastify.plugin.spec.js index 3df39c87b21..2db27074a31 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.fastify.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/sources/taint-tracking.fastify.plugin.spec.js @@ -122,7 +122,6 @@ describe('Path params sourcing with fastify', () => { reply.code(200).send() }) - await appInstance.listen({ port: 0 }) const port = appInstance.server.address().port @@ -132,4 +131,4 @@ describe('Path params sourcing with fastify', () => { expect(response.status).to.be.equal(200) }) }) -}) +})