Skip to content

Commit 68c5e0e

Browse files
IlyasShabirochdev
authored andcommitted
support blocking on fastify multipart (#5980)
1 parent 1446c8c commit 68c5e0e

File tree

3 files changed

+125
-12
lines changed

3 files changed

+125
-12
lines changed

packages/datadog-instrumentations/src/fastify.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const pathParamsReadCh = channel('datadog:fastify:path-params:finish')
1414

1515
const parsingResources = new WeakMap()
1616
const cookiesPublished = new WeakSet()
17+
const bodyPublished = new WeakSet()
1718

1819
function wrapFastify (fastify, hasParsingEvents) {
1920
if (typeof fastify !== 'function') return fastify
@@ -124,6 +125,20 @@ function preHandler (request, reply, done) {
124125
if (!reply || typeof reply.send !== 'function') return done()
125126

126127
const req = getReq(request)
128+
const res = getRes(reply)
129+
130+
const hasBody = request.body && Object.keys(request.body).length > 0
131+
132+
// For multipart/form-data, the body is not available until after preValidation hook
133+
if (bodyParserReadCh.hasSubscribers && hasBody && !bodyPublished.has(req)) {
134+
const abortController = new AbortController()
135+
136+
bodyParserReadCh.publish({ req, res, body: request.body, abortController })
137+
138+
bodyPublished.add(req)
139+
140+
if (abortController.signal.aborted) return
141+
}
127142

128143
reply.send = wrapSend(reply.send, req)
129144

@@ -151,11 +166,14 @@ function preValidation (request, reply, done) {
151166
if (abortController.signal.aborted) return
152167
}
153168

154-
if (bodyParserReadCh.hasSubscribers && request.body) {
169+
// Analyze body before schema validation
170+
if (bodyParserReadCh.hasSubscribers && request.body && !bodyPublished.has(req)) {
155171
abortController ??= new AbortController()
156172

157173
bodyParserReadCh.publish({ req, res, body: request.body, abortController })
158174

175+
bodyPublished.add(req)
176+
159177
if (abortController.signal.aborted) return
160178
}
161179

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

Lines changed: 102 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ const { assert } = require('chai')
55
const path = require('path')
66
const zlib = require('zlib')
77
const fs = require('node:fs')
8+
const semver = require('semver')
89
const agent = require('../plugins/agent')
910
const appsec = require('../../src/appsec')
1011
const Config = require('../../src/config')
1112
const { json } = require('../../src/appsec/blocked_templates')
1213

13-
withVersions('fastify', 'fastify', '>=2', version => {
14+
withVersions('fastify', 'fastify', '>=2', (fastifyVersion, _, fastifyLoadedVersion) => {
1415
describe('Suspicious request blocking - query', () => {
1516
let server, requestBody, axios
1617

@@ -19,7 +20,7 @@ withVersions('fastify', 'fastify', '>=2', version => {
1920
})
2021

2122
before((done) => {
22-
const fastify = require(`../../../../versions/fastify@${version}`).get()
23+
const fastify = require(`../../../../versions/fastify@${fastifyVersion}`).get()
2324

2425
const app = fastify()
2526

@@ -84,7 +85,7 @@ withVersions('fastify', 'fastify', '>=2', version => {
8485
})
8586

8687
before((done) => {
87-
const fastify = require(`../../../../versions/fastify@${version}`).get()
88+
const fastify = require(`../../../../versions/fastify@${fastifyVersion}`).get()
8889

8990
const app = fastify()
9091

@@ -184,7 +185,7 @@ withVersions('fastify', 'fastify', '>=2', version => {
184185
})
185186

186187
before((done) => {
187-
const fastify = require(`../../../../versions/fastify@${version}`).get()
188+
const fastify = require(`../../../../versions/fastify@${fastifyVersion}`).get()
188189

189190
const app = fastify()
190191

@@ -259,7 +260,7 @@ withVersions('fastify', 'fastify', '>=2', version => {
259260
})
260261

261262
before((done) => {
262-
const fastify = require(`../../../../versions/fastify@${version}`).get()
263+
const fastify = require(`../../../../versions/fastify@${fastifyVersion}`).get()
263264

264265
const app = fastify()
265266
app.get('/multiple-path-params/:parameter1/:parameter2', (request, reply) => {
@@ -441,21 +442,21 @@ withVersions('fastify', 'fastify', '>=2', version => {
441442
let server, requestCookie, axios
442443

443444
before(function () {
444-
if (version === '3.9.2') {
445+
if (semver.intersects(fastifyLoadedVersion, '3.9.2')) {
445446
// Fastify 3.9.2 is incompatible with @fastify/cookie >=6
446447
this.skip()
447448
}
448449

449450
// Skip preParsing hook for Fastify 2.x - has compatibility issues
450-
if (hook === 'preParsing' && version.startsWith('2')) {
451+
if (hook === 'preParsing' && semver.intersects(fastifyLoadedVersion, '2')) {
451452
this.skip()
452453
}
453454

454455
return agent.load(['fastify', '@fastify/cookie', 'http'], { client: false })
455456
})
456457

457458
before((done) => {
458-
const fastify = require(`../../../../versions/fastify@${version}`).get()
459+
const fastify = require(`../../../../versions/fastify@${fastifyVersion}`).get()
459460
const fastifyCookie = require(`../../../../versions/@fastify/cookie@${cookieVersion}`).get()
460461

461462
const app = fastify()
@@ -498,9 +499,7 @@ withVersions('fastify', 'fastify', '>=2', version => {
498499
})
499500

500501
after(() => {
501-
if (server) {
502-
server.close()
503-
}
502+
server?.close()
504503
return agent.close({ ritmReset: false })
505504
})
506505

@@ -530,6 +529,98 @@ withVersions('fastify', 'fastify', '>=2', version => {
530529
})
531530
})
532531
})
532+
533+
describe('Suspicious request blocking - multipart', () => {
534+
withVersions('fastify', '@fastify/multipart', (multipartVersion, _, multipartLoadedVersion) => {
535+
let server, uploadSpy, axios
536+
537+
// The skips in this section are complex because of the incompatibilities between Fastify and @fastify/multipart
538+
// We are not testing every major version of those libraries because of the complexity of the tests
539+
before(function () {
540+
// @fastify/multipart is not compatible with Fastify 2.x
541+
if (semver.intersects(fastifyLoadedVersion, '2')) {
542+
this.skip()
543+
}
544+
545+
// This Fastify version is working only with @fastify/multipart 6
546+
if (semver.intersects(fastifyLoadedVersion, '3.9.2') && semver.intersects(multipartLoadedVersion, '>=7')) {
547+
this.skip()
548+
}
549+
550+
// Fastify 5 drop le support pour multipart <7
551+
if (semver.intersects(fastifyLoadedVersion, '>=5') && semver.intersects(multipartLoadedVersion, '<7.0.0')) {
552+
this.skip()
553+
}
554+
555+
return agent.load(['fastify', '@fastify/multipart', 'http'], { client: false })
556+
})
557+
558+
before((done) => {
559+
const fastify = require(`../../../../versions/fastify@${fastifyVersion}`).get()
560+
const fastifyMultipart = require(`../../../../versions/@fastify/multipart@${multipartVersion}`).get()
561+
562+
const app = fastify()
563+
564+
app.register(fastifyMultipart, { attachFieldsToBody: true })
565+
566+
app.post('/', (request, reply) => {
567+
uploadSpy()
568+
reply.send('DONE')
569+
})
570+
571+
app.listen({ port: 0 }, () => {
572+
const port = server.address().port
573+
axios = Axios.create({ baseURL: `http://localhost:${port}` })
574+
done()
575+
})
576+
server = app.server
577+
})
578+
579+
beforeEach(() => {
580+
uploadSpy = sinon.stub()
581+
appsec.enable(new Config({
582+
appsec: {
583+
enabled: true,
584+
rules: path.join(__dirname, 'body-parser-rules.json')
585+
}
586+
}))
587+
})
588+
589+
afterEach(() => {
590+
appsec.disable()
591+
})
592+
593+
after(() => {
594+
server?.close()
595+
return agent.close({ ritmReset: false })
596+
})
597+
598+
it('should not block the request without an attack', async () => {
599+
const form = new FormData()
600+
form.append('key', 'value')
601+
602+
const res = await axios.post('/', form)
603+
604+
assert.strictEqual(res.status, 200)
605+
sinon.assert.calledOnce(uploadSpy)
606+
assert.strictEqual(res.data, 'DONE')
607+
})
608+
609+
it('should block the request when attack is detected', async () => {
610+
try {
611+
const form = new FormData()
612+
form.append('key', 'testattack')
613+
614+
await axios.post('/', form)
615+
616+
return Promise.reject(new Error('Request should not return 200'))
617+
} catch (e) {
618+
assert.strictEqual(e.response.status, 403)
619+
sinon.assert.notCalled(uploadSpy)
620+
}
621+
})
622+
})
623+
})
533624
})
534625

535626
describe('Api Security - Fastify', () => {

packages/dd-trace/test/plugins/externals.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@
161161
{
162162
"name": "@fastify/cookie",
163163
"versions": [">=6"]
164+
},
165+
{
166+
"name": "@fastify/multipart",
167+
"versions": [">=6"]
164168
}
165169
],
166170
"generic-pool": [

0 commit comments

Comments
 (0)