From 8b7b72111e4f3a8d0608bc484622fc337a0578fd Mon Sep 17 00:00:00 2001 From: simon-id Date: Mon, 25 Sep 2023 16:26:12 +0200 Subject: [PATCH 1/4] add path params waf listener --- packages/dd-trace/src/appsec/channels.js | 1 + packages/dd-trace/src/appsec/index.js | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index b2058f04482..a2955418086 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -9,6 +9,7 @@ module.exports = { graphqlFinishExecute: dc.channel('apm:graphql:execute:finish'), incomingHttpRequestStart: dc.channel('dd-trace:incomingHttpRequestStart'), incomingHttpRequestEnd: dc.channel('dd-trace:incomingHttpRequestEnd'), + paramsParser: dc.channel('datadog:express:process_params:start'), passportVerify: dc.channel('datadog:passport:verify:finish'), queryParser: dc.channel('datadog:query:read:finish'), setCookieChannel: dc.channel('datadog:iast:set-cookie') diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index 46d95f0f38c..82243c362b0 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -9,6 +9,7 @@ const { graphqlFinishExecute, incomingHttpRequestStart, incomingHttpRequestEnd, + paramsParser, passportVerify, queryParser } = require('./channels') @@ -45,6 +46,7 @@ function enable (_config) { bodyParser.subscribe(onRequestBodyParsed) queryParser.subscribe(onRequestQueryParsed) cookieParser.subscribe(onRequestCookieParser) + paramsParser.subscribe(onRequestParamsParsed) graphqlFinishExecute.subscribe(onGraphqlFinishExecute) if (_config.appsec.eventTracking.enabled) { @@ -163,6 +165,19 @@ function onRequestCookieParser ({ req, res, abortController, cookies }) { handleResults(results, req, res, rootSpan, abortController) } +function onRequestParamsParsed ({ req, res, abortController, params }) { + const rootSpan = web.root(req) + if (!rootSpan) return + + if (!params || typeof params !== 'object' || !Object.keys(params).length) return + + const results = waf.run({ + [addresses.HTTP_INCOMING_PARAMS]: params + }, req) + + handleResults(results, req, res, rootSpan, abortController) +} + function onPassportVerify ({ credentials, user }) { const store = storage.getStore() const rootSpan = store && store.req && web.root(store.req) @@ -214,6 +229,7 @@ function disable () { if (incomingHttpRequestEnd.hasSubscribers) incomingHttpRequestEnd.unsubscribe(incomingHttpEndTranslator) if (queryParser.hasSubscribers) queryParser.unsubscribe(onRequestQueryParsed) if (cookieParser.hasSubscribers) cookieParser.unsubscribe(onRequestCookieParser) + if (paramsParser.hasSubscribers) paramsParser.unsubscribe(onRequestParamsParsed) if (passportVerify.hasSubscribers) passportVerify.unsubscribe(onPassportVerify) } From 90a30b23e03e12e355ce6385ce6206dcecac5b93 Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 29 Sep 2023 09:40:18 +0200 Subject: [PATCH 2/4] convert arrow function to function declaration --- packages/datadog-instrumentations/src/express.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/datadog-instrumentations/src/express.js b/packages/datadog-instrumentations/src/express.js index f16c79f3d40..265509eeb9c 100644 --- a/packages/datadog-instrumentations/src/express.js +++ b/packages/datadog-instrumentations/src/express.js @@ -59,7 +59,7 @@ addHook({ }) const processParamsStartCh = channel('datadog:express:process_params:start') -const wrapProcessParamsMethod = (requestPositionInArguments) => { +function wrapProcessParamsMethod (requestPositionInArguments) { return (original) => { return function () { if (processParamsStartCh.hasSubscribers) { From 7e78b277e89cf380ba1f0a918370b340f9011a1e Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 29 Sep 2023 17:26:24 +0200 Subject: [PATCH 3/4] update instrumentation to support blocking --- packages/datadog-instrumentations/src/express.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/datadog-instrumentations/src/express.js b/packages/datadog-instrumentations/src/express.js index 265509eeb9c..172e0a4587a 100644 --- a/packages/datadog-instrumentations/src/express.js +++ b/packages/datadog-instrumentations/src/express.js @@ -63,7 +63,17 @@ function wrapProcessParamsMethod (requestPositionInArguments) { return (original) => { return function () { if (processParamsStartCh.hasSubscribers) { - processParamsStartCh.publish({ req: arguments[requestPositionInArguments] }) + const req = arguments[requestPositionInArguments] + const abortController = new AbortController() + + processParamsStartCh.publish({ + req, + res: req?.res, + abortController, + params: req?.params + }) + + if (abortController.signal.aborted) return } return original.apply(this, arguments) From fa6e8a03c21b887e638024e1de9f49c59eba4013 Mon Sep 17 00:00:00 2001 From: simon-id Date: Wed, 4 Oct 2023 00:47:23 +0200 Subject: [PATCH 4/4] push some test code --- packages/dd-trace/test/appsec/index.spec.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js index 643ad0c3c6d..87f9dea41f1 100644 --- a/packages/dd-trace/test/appsec/index.spec.js +++ b/packages/dd-trace/test/appsec/index.spec.js @@ -10,6 +10,7 @@ const { graphqlFinishExecute, incomingHttpRequestStart, incomingHttpRequestEnd, + paramsParser, queryParser, passportVerify } = require('../../src/appsec/channels') @@ -125,6 +126,7 @@ describe('AppSec Index', () => { it('should subscribe to blockable channels', () => { expect(bodyParser.hasSubscribers).to.be.false expect(cookieParser.hasSubscribers).to.be.false + expect(paramsParser.hasSubscribers).to.be.false expect(queryParser.hasSubscribers).to.be.false expect(passportVerify.hasSubscribers).to.be.false expect(graphqlFinishExecute.hasSubscribers).to.be.false @@ -134,6 +136,7 @@ describe('AppSec Index', () => { expect(bodyParser.hasSubscribers).to.be.true expect(cookieParser.hasSubscribers).to.be.true expect(graphqlFinishExecute.hasSubscribers).to.be.true + expect(paramsParser.hasSubscribers).to.be.true expect(queryParser.hasSubscribers).to.be.true expect(passportVerify.hasSubscribers).to.be.true }) @@ -196,6 +199,7 @@ describe('AppSec Index', () => { expect(bodyParser.hasSubscribers).to.be.false expect(cookieParser.hasSubscribers).to.be.false expect(graphqlFinishExecute.hasSubscribers).to.be.false + expect(paramsParser.hasSubscribers).to.be.false expect(queryParser.hasSubscribers).to.be.false expect(passportVerify.hasSubscribers).to.be.false }) @@ -614,7 +618,7 @@ describe('AppSec Index', () => { }) it('Should not call waf if req is unavailable', () => { - const resolvers = { user: [ { id: '1234' } ] } + const resolvers = { user: [{ id: '1234' }] } sinon.stub(waf, 'run') sinon.stub(storage, 'getStore').returns({}) @@ -626,7 +630,7 @@ describe('AppSec Index', () => { it('Should call waf if resolvers is well formatted', () => { const context = { resolvers: { - user: [ { id: '1234' } ] + user: [{ id: '1234' }] } } const rootSpan = {}