diff --git a/packages/dd-trace/src/appsec/blocking.js b/packages/dd-trace/src/appsec/blocking.js index 79ce4976f72..f22d93116db 100644 --- a/packages/dd-trace/src/appsec/blocking.js +++ b/packages/dd-trace/src/appsec/blocking.js @@ -14,6 +14,8 @@ let defaultBlockingActionParameters const responseBlockedSet = new WeakSet() +const blockDelegations = new WeakMap() + const specificBlockingTypes = { GRAPHQL: 'graphql' } @@ -99,6 +101,9 @@ function getBlockingData (req, specificType, actionParameters) { } function block (req, res, rootSpan, abortController, actionParameters = defaultBlockingActionParameters) { + // synchronous blocking overrides previously created delegations + blockDelegations.delete(res) + try { if (res.headersSent) { log.warn('[ASM] Cannot send blocking response when headers have already been sent') @@ -127,11 +132,33 @@ function block (req, res, rootSpan, abortController, actionParameters = defaultB rootSpan?.setTag('_dd.appsec.block.failed', 1) log.error('[ASM] Blocking error', err) + // TODO: if blocking fails, then the response will never be sent ? + updateBlockFailureMetric(req) return false } } +function registerBlockDelegation (req, res) { + const args = arguments + + return new Promise((resolve) => { + // ignore subsequent delegations by never calling their resolve() + if (blockDelegations.has(res)) return + + blockDelegations.set(res, { args, resolve }) + }) +} + +function callBlockDelegation (res) { + const delegation = blockDelegations.get(res) + if (delegation) { + const result = block.apply(this, delegation.args) + delegation.resolve(result) + return result + } +} + function getBlockingAction (actions) { // waf only returns one action, but it prioritizes redirect over block return actions?.redirect_request || actions?.block_request @@ -158,6 +185,8 @@ function setDefaultBlockingActionParameters (actions) { module.exports = { addSpecificEndpoint, block, + registerBlockDelegation, + callBlockDelegation, specificBlockingTypes, getBlockingData, getBlockingAction, diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index 232df028ff3..cfe23d63bd9 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -14,10 +14,11 @@ module.exports = { expressProcessParams: dc.channel('datadog:express:process_params:start'), expressSession: dc.channel('datadog:express-session:middleware:finish'), fastifyBodyParser: dc.channel('datadog:fastify:body-parser:finish'), - fastifyResponseChannel: dc.channel('datadog:fastify:response:finish'), - fastifyQueryParams: dc.channel('datadog:fastify:query-params:finish'), fastifyCookieParser: dc.channel('datadog:fastify-cookie:read:finish'), + fastifyMiddlewareError: dc.channel('apm:fastify:middleware:error'), fastifyPathParams: dc.channel('datadog:fastify:path-params:finish'), + fastifyQueryParams: dc.channel('datadog:fastify:query-params:finish'), + fastifyResponseChannel: dc.channel('datadog:fastify:response:finish'), fsOperationStart: dc.channel('apm:fs:operation:start'), graphqlMiddlewareChannel: dc.tracingChannel('datadog:apollo:middleware'), httpClientRequestStart: dc.channel('apm:http:client:request:start'), diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index cb76ab64b2b..a4505584562 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -34,7 +34,7 @@ const apiSecuritySampler = require('./api_security_sampler') const web = require('../plugins/util/web') const { extractIp } = require('../plugins/util/ip_extractor') const { HTTP_CLIENT_IP } = require('../../../../ext/tags') -const { isBlocked, block, setTemplates, getBlockingAction } = require('./blocking') +const { isBlocked, block, callBlockDelegation, setTemplates, getBlockingAction } = require('./blocking') const UserTracking = require('./user_tracking') const { storage } = require('../../../datadog-core') const graphql = require('./graphql') @@ -306,8 +306,13 @@ function onResponseWriteHead ({ req, res, abortController, statusCode, responseH storedResponseHeaders.set(req, responseHeaders) } + // TODO: do not call waf if inside block() + // if (isBlocking()) { + // return + // } + // avoid "write after end" error - if (isBlocked(res)) { + if (isBlocked(res) || callBlockDelegation(res)) { abortController?.abort() return } diff --git a/packages/dd-trace/src/appsec/rasp/fs-plugin.js b/packages/dd-trace/src/appsec/rasp/fs-plugin.js index 5d8de0cbac3..cf4cd31f805 100644 --- a/packages/dd-trace/src/appsec/rasp/fs-plugin.js +++ b/packages/dd-trace/src/appsec/rasp/fs-plugin.js @@ -35,6 +35,7 @@ class AppsecFsPlugin extends Plugin { this.addBind('apm:fs:operation:finish', this._onFsOperationFinishOrRenderEnd) this.addBind('tracing:datadog:express:response:render:start', this._onResponseRenderStart) this.addBind('tracing:datadog:express:response:render:end', this._onFsOperationFinishOrRenderEnd) + // We might have to add the same subscribers for fastify later super.configure(true) } diff --git a/packages/dd-trace/src/appsec/rasp/index.js b/packages/dd-trace/src/appsec/rasp/index.js index 6da75c91283..7f941d5d5f2 100644 --- a/packages/dd-trace/src/appsec/rasp/index.js +++ b/packages/dd-trace/src/appsec/rasp/index.js @@ -1,8 +1,12 @@ 'use strict' const web = require('../../plugins/util/web') -const { setUncaughtExceptionCaptureCallbackStart, expressMiddlewareError } = require('../channels') -const { block, isBlocked } = require('../blocking') +const { + setUncaughtExceptionCaptureCallbackStart, + expressMiddlewareError, + fastifyMiddlewareError +} = require('../channels') +const { block, registerBlockDelegation, isBlocked } = require('../blocking') const ssrf = require('./ssrf') const sqli = require('./sql_injection') const lfi = require('./lfi') @@ -39,7 +43,7 @@ function findDatadogRaspAbortError (err, deep = 10) { } function handleUncaughtExceptionMonitor (error) { - if (!blockOnDatadogRaspAbortError({ error })) return + if (!blockOnDatadogRaspAbortError({ error, isTopLevel: true })) return if (process.hasUncaughtExceptionCaptureCallback()) { // uncaughtException event is not executed when hasUncaughtExceptionCaptureCallback is true @@ -81,15 +85,21 @@ function handleUncaughtExceptionMonitor (error) { } } -function blockOnDatadogRaspAbortError ({ error }) { +function blockOnDatadogRaspAbortError ({ error, isTopLevel }) { const abortError = findDatadogRaspAbortError(error) if (!abortError) return false const { req, res, blockingAction, raspRule, ruleTriggered } = abortError if (!isBlocked(res)) { - const blocked = block(req, res, web.root(req), null, blockingAction) + const blockFn = isTopLevel ? block : registerBlockDelegation + const blocked = blockFn(req, res, web.root(req), null, blockingAction) if (ruleTriggered) { - updateRaspRuleMatchMetricTags(req, raspRule, true, blocked) + // block() returns a bool, and registerBlockDelegation() returns a promise + // we use Promise.resolve() to handle both cases + Promise.resolve(blocked).then(blocked => { + // TODO: bug: this should probably be called for each match even without block + updateRaspRuleMatchMetricTags(req, raspRule, true, blocked) + }) } } @@ -103,7 +113,9 @@ function enable (config) { cmdi.enable(config) process.on('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor) + expressMiddlewareError.subscribe(blockOnDatadogRaspAbortError) + fastifyMiddlewareError.subscribe(blockOnDatadogRaspAbortError) } function disable () { @@ -113,7 +125,9 @@ function disable () { cmdi.disable() process.off('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor) - if (expressMiddlewareError.hasSubscribers) expressMiddlewareError.unsubscribe(blockOnDatadogRaspAbortError) + + expressMiddlewareError.unsubscribe(blockOnDatadogRaspAbortError) + fastifyMiddlewareError.unsubscribe(blockOnDatadogRaspAbortError) } module.exports = { diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index f3b3cea6ba9..cb206135af5 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -19,6 +19,11 @@ const RULE_TYPES = { SSRF: 'ssrf' } +const ALLOWED_ROOTSPAN_NAMES = new Set([ + 'express.request', + 'fastify.request' +]) + class DatadogRaspAbortError extends Error { constructor (req, res, blockingAction, raspRule, ruleTriggered) { super('DatadogRaspAbortError') @@ -28,6 +33,12 @@ class DatadogRaspAbortError extends Error { this.blockingAction = blockingAction this.raspRule = raspRule this.ruleTriggered = ruleTriggered + + // hide these props to not pollute app logs + Object.defineProperties(this, { + req: { enumerable: false }, + res: { enumerable: false } + }) } } @@ -52,9 +63,9 @@ function handleResult (result, req, res, abortController, config, raspRule) { if (abortController && !abortOnUncaughtException) { const blockingAction = getBlockingAction(result?.actions) + const rootSpanName = rootSpan?.context?.()?._name - // Should block only in express - if (blockingAction && rootSpan?.context()._name === 'express.request') { + if (blockingAction && ALLOWED_ROOTSPAN_NAMES.has(rootSpanName)) { const abortError = new DatadogRaspAbortError(req, res, blockingAction, raspRule, ruleTriggered) abortController.abort(abortError) diff --git a/packages/dd-trace/test/appsec/blocking.spec.js b/packages/dd-trace/test/appsec/blocking.spec.js index 9ed222fd339..148dbf7c357 100644 --- a/packages/dd-trace/test/appsec/blocking.spec.js +++ b/packages/dd-trace/test/appsec/blocking.spec.js @@ -16,7 +16,7 @@ describe('blocking', () => { } let log, telemetry - let block, setTemplates + let block, registerBlockDelegation, callBlockDelegation, setTemplates let req, res, rootSpan beforeEach(() => { @@ -35,6 +35,8 @@ describe('blocking', () => { }) block = blocking.block + registerBlockDelegation = blocking.registerBlockDelegation + callBlockDelegation = blocking.callBlockDelegation setTemplates = blocking.setTemplates req = { @@ -151,6 +153,102 @@ describe('blocking', () => { }) }) + describe('block delegation', () => { + it('should delegate block', (done) => { + setTemplates(config) + + const abortController = new AbortController() + const promise = registerBlockDelegation(req, res, rootSpan, abortController) + + expect(rootSpan.setTag).to.not.have.been.called + expect(res.writeHead).to.not.have.been.called + expect(res.constructor.prototype.end).to.not.have.been.called + expect(abortController.signal.aborted).to.be.false + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called + + const blocked = callBlockDelegation(res) + + expect(blocked).to.be.true + expect(rootSpan.setTag).to.have.been.calledOnceWithExactly('appsec.blocked', 'true') + expect(res.writeHead).to.have.been.calledOnceWithExactly(403, { + 'Content-Type': 'application/json', + 'Content-Length': 8 + }) + expect(res.constructor.prototype.end).to.have.been.calledOnceWithExactly('jsonBody') + expect(abortController.signal.aborted).to.be.true + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called + + promise.then(blocked => { + expect(blocked).to.be.true + done() + }) + }) + + it('should only resolve the first blocking delegation per request', (done) => { + const firstPromise = registerBlockDelegation(req, res, rootSpan) + const secondPromise = sinon.stub() + const thirdPromise = sinon.stub() + registerBlockDelegation(req, res, rootSpan).then(secondPromise) + registerBlockDelegation(req, res, rootSpan).then(thirdPromise) + + const blocked = callBlockDelegation(res) + + expect(blocked).to.be.true + expect(rootSpan.setTag).to.have.been.calledOnce + expect(res.writeHead).to.have.been.calledOnce + expect(res.constructor.prototype.end).to.have.been.calledOnce + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called + + firstPromise.then((blocked) => { + expect(blocked).to.be.true + + setTimeout(() => { + expect(secondPromise).to.not.have.been.called + expect(thirdPromise).to.not.have.been.called + done() + }, 100) + }) + }) + + it('should do nothing if no blocking delegation exists', () => { + const blocked = callBlockDelegation(res) + + expect(blocked).to.not.be.ok + expect(log.warn).to.not.have.been.called + expect(rootSpan.setTag).to.not.have.been.called + expect(res.writeHead).to.not.have.been.called + expect(res.constructor.prototype.end).to.not.have.been.called + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called + }) + + it('should cancel block delegations when block is called', (done) => { + const promise = sinon.stub() + + registerBlockDelegation(req, res, rootSpan).then(promise) + + const blocked = block(req, res, rootSpan) + + expect(blocked).to.be.true + expect(rootSpan.setTag).to.have.been.calledOnce + expect(res.writeHead).to.have.been.calledOnce + expect(res.constructor.prototype.end).to.have.been.calledOnce + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called + + const result = callBlockDelegation(res) + + expect(result).to.not.be.ok + expect(rootSpan.setTag).to.have.been.calledOnce + expect(res.writeHead).to.have.been.calledOnce + expect(res.constructor.prototype.end).to.have.been.calledOnce + expect(telemetry.updateBlockFailureMetric).to.not.have.been.called + + setTimeout(() => { + expect(promise).to.not.have.been.called + done() + }, 100) + }) + }) + describe('block with default templates', () => { const config = { appsec: { diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js index 2d314ad31c6..d5c64fed78d 100644 --- a/packages/dd-trace/test/appsec/index.spec.js +++ b/packages/dd-trace/test/appsec/index.spec.js @@ -98,7 +98,8 @@ describe('AppSec Index', function () { } blocking = { - setTemplates: sinon.stub() + setTemplates: sinon.stub(), + callBlockDelegation: sinon.stub() } UserTracking = { @@ -1122,6 +1123,7 @@ describe('AppSec Index', function () { }, req) expect(abortController.abort).to.have.been.calledOnce expect(res.constructor.prototype.end).to.have.been.calledOnce + expect(blocking.callBlockDelegation).to.have.been.calledOnce abortController.abort.resetHistory() @@ -1130,6 +1132,17 @@ describe('AppSec Index', function () { expect(waf.run).to.have.been.calledOnce expect(abortController.abort).to.have.been.calledOnce expect(res.constructor.prototype.end).to.have.been.calledOnce + expect(blocking.callBlockDelegation).to.have.been.calledOnce + }) + + it('should call abortController if blocking delegate is successful', () => { + blocking.callBlockDelegation.returns(true) + + responseWriteHead.publish({ req, res, abortController, statusCode: 404, responseHeaders: {} }) + + expect(blocking.callBlockDelegation).to.have.been.calledOnceWithExactly(res) + expect(abortController.abort).to.have.been.calledOnce + expect(waf.run).to.not.have.been.called }) it('should not call the WAF if response was already analyzed', () => { diff --git a/packages/dd-trace/test/appsec/rasp/index.spec.js b/packages/dd-trace/test/appsec/rasp/index.spec.js index 9785444caad..4cbeee95a6c 100644 --- a/packages/dd-trace/test/appsec/rasp/index.spec.js +++ b/packages/dd-trace/test/appsec/rasp/index.spec.js @@ -1,11 +1,10 @@ 'use strict' const proxyquire = require('proxyquire') -const { handleUncaughtExceptionMonitor } = require('../../../src/appsec/rasp') const { DatadogRaspAbortError } = require('../../../src/appsec/rasp/utils') describe('RASP', () => { - let rasp, subscribe, unsubscribe, block, blocked, updateRaspRuleMatchMetricTags + let rasp, channels, blocking, blocked, updateRaspRuleMatchMetricTags beforeEach(() => { const config = { @@ -18,24 +17,32 @@ describe('RASP', () => { } } - subscribe = sinon.stub() - unsubscribe = sinon.stub() + channels = { + expressMiddlewareError: { + subscribe: sinon.stub(), + unsubscribe: sinon.stub(), + hasSubscribers: true + }, + fastifyMiddlewareError: { + subscribe: sinon.stub(), + unsubscribe: sinon.stub(), + hasSubscribers: true + } + } + + blocked = false + + blocking = { + block: sinon.stub().returns(true), + registerBlockDelegation: sinon.stub().resolves(true), + isBlocked: sinon.stub().callsFake(() => blocked) + } - block = sinon.stub().returns(true) updateRaspRuleMatchMetricTags = sinon.stub() rasp = proxyquire('../../../src/appsec/rasp', { - '../blocking': { - block, - isBlocked: sinon.stub().callsFake(() => blocked) - }, - '../channels': { - expressMiddlewareError: { - subscribe, - unsubscribe, - hasSubscribers: true - } - }, + '../blocking': blocking, + '../channels': channels, '../telemetry': { updateRaspRuleMatchMetricTags } @@ -54,19 +61,21 @@ describe('RASP', () => { const err = new Error() err.cause = err - handleUncaughtExceptionMonitor(err) + rasp.handleUncaughtExceptionMonitor(err) }) }) describe('enable/disable', () => { - it('should subscribe to apm:express:middleware:error', () => { - sinon.assert.calledOnce(subscribe) + it('should subscribe to error channels', () => { + sinon.assert.calledOnce(channels.expressMiddlewareError.subscribe) + sinon.assert.calledOnce(channels.fastifyMiddlewareError.subscribe) }) - it('should unsubscribe to apm:express:middleware:error', () => { + it('should unsubscribe from error channels', () => { rasp.disable() - sinon.assert.calledOnce(unsubscribe) + sinon.assert.calledOnce(channels.expressMiddlewareError.unsubscribe) + sinon.assert.calledOnce(channels.fastifyMiddlewareError.unsubscribe) }) }) @@ -87,20 +96,25 @@ describe('RASP', () => { it('should skip non DatadogRaspAbortError', () => { rasp.blockOnDatadogRaspAbortError({ error: new Error() }) - sinon.assert.notCalled(block) + sinon.assert.notCalled(blocking.block) + sinon.assert.notCalled(blocking.registerBlockDelegation) sinon.assert.notCalled(updateRaspRuleMatchMetricTags) }) - it('should block DatadogRaspAbortError first time and update metrics', () => { + it('should block DatadogRaspAbortError first time and update metrics', (done) => { rasp.blockOnDatadogRaspAbortError({ error: new DatadogRaspAbortError(req, res, blockingAction, raspRule, true) }) - sinon.assert.calledOnce(block) - sinon.assert.calledOnceWithExactly(updateRaspRuleMatchMetricTags, req, raspRule, true, true) + sinon.assert.calledOnce(blocking.registerBlockDelegation) + + setImmediate(() => { + sinon.assert.calledOnceWithExactly(updateRaspRuleMatchMetricTags, req, raspRule, true, true) + done() + }) }) - it('should skip calling block if blocked before', () => { + it('should skip calling block if blocked before', (done) => { rasp.blockOnDatadogRaspAbortError({ error: new DatadogRaspAbortError(req, res, blockingAction, raspRule, true) }) @@ -114,8 +128,23 @@ describe('RASP', () => { error: new DatadogRaspAbortError(req, res, blockingAction, raspRule, true) }) - sinon.assert.calledOnce(block) - sinon.assert.calledOnce(updateRaspRuleMatchMetricTags) + sinon.assert.calledOnce(blocking.registerBlockDelegation) + + setImmediate(() => { + sinon.assert.calledOnce(updateRaspRuleMatchMetricTags) + done() + }) + }) + + it('should block without delegate when called by handleUncaughtExceptionMonitor', (done) => { + rasp.handleUncaughtExceptionMonitor(new DatadogRaspAbortError(req, res, blockingAction, raspRule, true)) + + sinon.assert.calledOnce(blocking.block) + + setImmediate(() => { + sinon.assert.calledOnceWithExactly(updateRaspRuleMatchMetricTags, req, raspRule, true, true) + done() + }) }) }) }) diff --git a/packages/dd-trace/test/appsec/rasp/rasp_blocking.fastify.plugin.spec.js b/packages/dd-trace/test/appsec/rasp/rasp_blocking.fastify.plugin.spec.js new file mode 100644 index 00000000000..a21f84907d0 --- /dev/null +++ b/packages/dd-trace/test/appsec/rasp/rasp_blocking.fastify.plugin.spec.js @@ -0,0 +1,179 @@ +'use strict' + +const path = require('path') +const agent = require('../../plugins/agent') +const Config = require('../../../src/config') +const appsec = require('../../../src/appsec') +const Axios = require('axios') +const { withVersions } = require('../../setup/mocha') +const { assert } = require('chai') +const { json: blockedJson } = require('../../../src/appsec/blocked_templates') +const { checkRaspExecutedAndNotThreat, checkRaspExecutedAndHasThreat } = require('./utils') + +describe('RASP - fastify blocking', () => { + withVersions('fastify', 'fastify', '>=2', (version) => { + let app, hooks, axios + + before(async () => { + await agent.load(['http', 'fastify'], { client: false }) + + appsec.enable(new Config({ + appsec: { + enabled: true, + rules: path.join(__dirname, 'resources', 'rasp_rules.json'), + rasp: { enabled: true } + } + })) + + const fastify = require(`../../../../../versions/fastify@${version}`).get() + app = fastify() + + hooks = { + onSend: sinon.stub().resolves(), + onResponse: sinon.stub().resolves(), + onError: sinon.stub().resolves() + } + + for (const [k, v] of Object.entries(hooks)) { + app.addHook(k, v) + } + + const childProcess = require('child_process') + const fs = require('fs') + const pg = require('../../../../../versions/pg@8.7.3').get() + const pool = new pg.Pool({ + host: '127.0.0.1', + user: 'postgres', + password: 'postgres', + database: 'postgres', + application_name: 'test' + }) + const http = require('http') + + app.get('/error', async (request, reply) => { + throw new Error('loul') + }) + + app.get('/cmdi', async (request, reply) => { + return childProcess.execFileSync('sh', ['-c', request.query.payload]).toString() + }) + + app.get('/shi', async (request, reply) => { + return childProcess.execSync(`ls ${request.query.payload ?? ''}`).toString() + }) + + app.get('/lfi', async (request, reply) => { + return fs.readFileSync(request.query.payload) + }) + + app.get('/sqli', async (request, reply) => { + return pool.query(`SELECT * FROM users WHERE id='${request.query.payload}'`) + }) + + app.get('/ssrf', async (request, reply) => { + await new Promise((resolve, reject) => { + http.get(`http://${request.query.payload}`, () => { + resolve() + }) + }) + }) + + await app.listen() + + axios = Axios.create({ + baseURL: `http://localhost:${app.server.address().port}`, + validateStatus: () => true, + responseType: 'text' + }) + }) + + afterEach(() => { + sinon.resetHistory() + }) + + after(async () => { + await app.server.close() + appsec.disable() + await agent.close({ ritmReset: false }) + }) + + it('should not block on user error', async () => { + const res = await axios('/error') + + sinon.assert.calledOnce(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.calledOnce(hooks.onError) + assert.strictEqual(res.status, 500) + assert.notStrictEqual(res.data, blockedJson) + assert(res.data.includes('loul')) + await checkRaspExecutedAndNotThreat(agent, false) + }) + + it('should not block without attack', async () => { + const res = await axios('/shi') + + sinon.assert.calledOnce(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.notCalled(hooks.onError) + assert.strictEqual(res.status, 200) + assert.notStrictEqual(res.data, blockedJson) + await checkRaspExecutedAndNotThreat(agent, true) + }) + + it('should block with CMDI', async () => { + const res = await axios('/cmdi?payload=cat /etc/passwd') + + sinon.assert.calledOnce(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.calledOnce(hooks.onError) + assert.strictEqual(res.status, 403) + assert.strictEqual(res.data, blockedJson) + await checkRaspExecutedAndHasThreat(agent, 'rasp-command_injection-rule-id-4') + }) + + it('should block with SHI', async () => { + const res = await axios('/shi?payload=$(cat /etc/passwd 1>%262 ; echo .)') + + sinon.assert.calledOnce(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.calledOnce(hooks.onError) + assert.strictEqual(res.status, 403) + assert.strictEqual(res.data, blockedJson) + await checkRaspExecutedAndHasThreat(agent, 'rasp-command_injection-rule-id-3') + }) + + it('should block with LFI', async () => { + const res = await axios('/lfi?payload=/etc/passwd') + + sinon.assert.calledOnce(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.calledOnce(hooks.onError) + assert.strictEqual(res.status, 403) + assert.strictEqual(res.data, blockedJson) + await checkRaspExecutedAndHasThreat(agent, 'rasp-lfi-rule-id-5') + }) + + it('should block with SQLI', async () => { + const res = await axios('/sqli?payload=\' OR 1 = 1 --') + + sinon.assert.calledOnce(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.calledOnce(hooks.onError) + assert.strictEqual(res.status, 403) + assert.strictEqual(res.data, blockedJson) + await checkRaspExecutedAndHasThreat(agent, 'rasp-sqli-rule-id-2') + }) + + it('should block with SSRF', async () => { + const res = await axios('/ssrf?payload=169.254.169.254') + + // some hooks won't be called because SSRF is blocked out of band + sinon.assert.notCalled(hooks.onSend) + sinon.assert.calledOnce(hooks.onResponse) + sinon.assert.notCalled(hooks.onError) + assert.strictEqual(res.status, 403) + assert.strictEqual(res.data, blockedJson) + await checkRaspExecutedAndHasThreat(agent, 'rasp-ssrf-rule-id-1') + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json b/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json index c0396bd9871..06a9c29dfbe 100644 --- a/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json +++ b/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json @@ -205,6 +205,59 @@ "block", "stack_trace" ] + }, + { + "id": "rasp-lfi-rule-id-5", + "name": "Local file inclusion exploit", + "enabled": true, + "tags": { + "type": "lfi", + "category": "vulnerability_trigger", + "cwe": "22", + "capec": "1000/255/153/126", + "confidence": "0", + "module": "rasp" + }, + "conditions": [ + { + "parameters": { + "resource": [ + { + "address": "server.io.fs.file" + } + ], + "params": [ + { + "address": "server.request.query" + }, + { + "address": "server.request.body" + }, + { + "address": "server.request.path_params" + }, + { + "address": "grpc.server.request.message" + }, + { + "address": "server.request.headers.no_cookies" + }, + { + "address": "graphql.server.all_resolvers" + }, + { + "address": "graphql.server.resolver" + } + ] + }, + "operator": "lfi_detector" + } + ], + "transformers": [], + "on_match": [ + "block", + "stack_trace" + ] } ] } diff --git a/packages/dd-trace/test/appsec/rasp/ssrf.express.plugin.spec.js b/packages/dd-trace/test/appsec/rasp/ssrf.express.plugin.spec.js index 1a853e2022f..ceeb372a29f 100644 --- a/packages/dd-trace/test/appsec/rasp/ssrf.express.plugin.spec.js +++ b/packages/dd-trace/test/appsec/rasp/ssrf.express.plugin.spec.js @@ -116,14 +116,19 @@ describe('RASP - ssrf', () => { withVersions('express', 'axios', axiosVersion => { let axiosToTest - beforeEach(() => { + beforeEach((done) => { axiosToTest = require(`../../../../../versions/axios@${axiosVersion}`).get() + + // we preload axios because it's lazyloading a debug dependency + // that in turns trigger LFI + axiosToTest.get('http://preloadaxios').catch(noop).then(done) }) it('Should not detect threat', async () => { app = (req, res) => { - axiosToTest.get(`https://${req.query.host}`).catch(noop) // swallow network error - res.end('end') + axiosToTest.get(`https://${req.query.host}`) + .catch(noop) // swallow network error + .then(() => res.end('end')) } await axios.get('/?host=www.datadoghq.com') diff --git a/packages/dd-trace/test/appsec/rasp/utils.spec.js b/packages/dd-trace/test/appsec/rasp/utils.spec.js index 14b39417bd0..92e3a4c2bdf 100644 --- a/packages/dd-trace/test/appsec/rasp/utils.spec.js +++ b/packages/dd-trace/test/appsec/rasp/utils.spec.js @@ -43,6 +43,31 @@ describe('RASP - utils.js', () => { } }) + function testAbortErrorInFramework (framework) { + const rootSpan = { + context: sinon.stub().returns({ _name: framework }) + } + const abortController = { + abort: sinon.stub(), + signal: {} + } + const result = { + actions: { + blocking_action: { type: 'block_request' } + } + } + + web.root.returns(rootSpan) + + utils.handleResult(result, req, res, abortController, config, raspRule) + + sinon.assert.calledOnce(abortController.abort) + const abortError = abortController.abort.firstCall.args[0] + expect(abortError).to.be.instanceOf(utils.DatadogRaspAbortError) + expect(abortError.raspRule).to.equal(raspRule) + expect(abortError.blockingAction).to.equal(result.actions.blocking_action) + } + describe('handleResult', () => { it('should report stack trace when generate_stack action is present in waf result', () => { const rootSpan = {} @@ -127,9 +152,17 @@ describe('RASP - utils.js', () => { sinon.assert.notCalled(stackTrace.reportStackTrace) }) - it('should create DatadogRaspAbortError when blockingAction is present', () => { + it('should create DatadogRaspAbortError when blockingAction is present in express', () => { + testAbortErrorInFramework('express.request') + }) + + it('should create DatadogRaspAbortError when blockingAction is present in fastify', () => { + testAbortErrorInFramework('fastify.request') + }) + + it('should not create DatadogRaspAbortError when blockingAction is present in an unsupported framework', () => { const rootSpan = { - context: sinon.stub().returns({ _name: 'express.request' }) + context: sinon.stub().returns({ _name: 'http.request' }) } const abortController = { abort: sinon.stub(), @@ -145,11 +178,7 @@ describe('RASP - utils.js', () => { utils.handleResult(result, req, res, abortController, config, raspRule) - sinon.assert.calledOnce(abortController.abort) - const abortError = abortController.abort.firstCall.args[0] - expect(abortError).to.be.instanceOf(utils.DatadogRaspAbortError) - expect(abortError.raspRule).to.equal(raspRule) - expect(abortError.blockingAction).to.equal(result.actions.blocking_action) + sinon.assert.notCalled(abortController.abort) }) it('should call updateRaspRuleMatchMetricTags when no blockingAction is present', () => { @@ -181,6 +210,9 @@ describe('RASP - utils.js', () => { expect(error.message).to.equal('DatadogRaspAbortError') expect(error.blockingAction).to.equal(blockingAction) expect(error.raspRule).to.equal(raspRule) + expect(error).to.have.property('req') + expect(error).to.have.property('res') + expect(Object.keys(error)).to.not.include.members(['req', 'res']) }) }) }) diff --git a/packages/dd-trace/test/plugins/externals.json b/packages/dd-trace/test/plugins/externals.json index 702584168cd..c3c42d8c0f4 100644 --- a/packages/dd-trace/test/plugins/externals.json +++ b/packages/dd-trace/test/plugins/externals.json @@ -183,6 +183,10 @@ { "name": "@fastify/multipart", "versions": [">=6"] + }, + { + "name": "pg", + "versions": ["8.7.3"] } ], "generic-pool": [