[ASM] multer instrumentation (#4781)

* multer instrumentation

* clean up test

* unsubscribe multer

* multer CI tests

* multer CI plugins tests

* Change multer version range

* Specify the version

* second try

* third

* multer request blocking integration test

* include iast tainting multer body test

* fix test

* Update integration-tests/multer.spec.js

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>

* Move taint multipart body test to the integration test

* delete test

* Move multer tests inside appsec

* Include test not using multer middleware

---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>

.github/workflows/appsec.yml

@@ -122,7 +122,7 @@ jobs:
   express:
     runs-on: ubuntu-latest
     env:
-      PLUGINS: express|body-parser|cookie-parser
+      PLUGINS: express|body-parser|cookie-parser|multer
     steps:
       - uses: actions/checkout@v4
       - uses: ./.github/actions/node/setup

integration-tests/appsec/multer.spec.js

@@ -0,0 +1,138 @@
+'use strict'
+
+const { assert } = require('chai')
+const path = require('path')
+const axios = require('axios')
+
+const {
+  createSandbox,
+  FakeAgent,
+  spawnProc
+} = require('../helpers')
+
+describe('multer', () => {
+  let sandbox, cwd, startupTestFile, agent, proc, env
+
+  ['1.4.4-lts.1', '1.4.5-lts.1'].forEach((version) => {
+    describe(`v${version}`, () => {
+      before(async () => {
+        sandbox = await createSandbox(['express', `multer@${version}`])
+        cwd = sandbox.folder
+        startupTestFile = path.join(cwd, 'appsec', 'multer', 'index.js')
+      })
+
+      after(async () => {
+        await sandbox.remove()
+      })
+
+      beforeEach(async () => {
+        agent = await new FakeAgent().start()
+
+        env = {
+          AGENT_PORT: agent.port,
+          DD_APPSEC_RULES: path.join(cwd, 'appsec', 'multer', 'body-parser-rules.json')
+        }
+
+        const execArgv = []
+
+        proc = await spawnProc(startupTestFile, { cwd, env, execArgv })
+      })
+
+      afterEach(async () => {
+        proc.kill()
+        await agent.stop()
+      })
+
+      describe('Suspicious request blocking', () => {
+        describe('using middleware', () => {
+          it('should not block the request without an attack', async () => {
+            const form = new FormData()
+            form.append('key', 'value')
+
+            const res = await axios.post(proc.url, form)
+
+            assert.equal(res.data, 'DONE')
+          })
+
+          it('should block the request when attack is detected', async () => {
+            try {
+              const form = new FormData()
+              form.append('key', 'testattack')
+
+              await axios.post(proc.url, form)
+
+              return Promise.reject(new Error('Request should not return 200'))
+            } catch (e) {
+              assert.equal(e.response.status, 403)
+            }
+          })
+        })
+
+        describe('not using middleware', () => {
+          it('should not block the request without an attack', async () => {
+            const form = new FormData()
+            form.append('key', 'value')
+
+            const res = await axios.post(`${proc.url}/no-middleware`, form)
+
+            assert.equal(res.data, 'DONE')
+          })
+
+          it('should block the request when attack is detected', async () => {
+            try {
+              const form = new FormData()
+              form.append('key', 'testattack')
+
+              await axios.post(`${proc.url}/no-middleware`, form)
+
+              return Promise.reject(new Error('Request should not return 200'))
+            } catch (e) {
+              assert.equal(e.response.status, 403)
+            }
+          })
+        })
+      })
+
+      describe('IAST', () => {
+        function assertCmdInjection ({ payload }) {
+          assert.isArray(payload)
+          assert.strictEqual(payload.length, 1)
+          assert.isArray(payload[0])
+
+          const { meta } = payload[0][0]
+
+          assert.property(meta, '_dd.iast.json')
+
+          const iastJson = JSON.parse(meta['_dd.iast.json'])
+
+          assert.isTrue(iastJson.vulnerabilities.some(v => v.type === 'COMMAND_INJECTION'))
+          assert.isTrue(iastJson.sources.some(s => s.origin === 'http.request.body'))
+        }
+
+        describe('using middleware', () => {
+          it('should taint multipart body', async () => {
+            const resultPromise = agent.assertMessageReceived(assertCmdInjection)
+
+            const formData = new FormData()
+            formData.append('command', 'echo 1')
+            await axios.post(`${proc.url}/cmd`, formData)
+
+            return resultPromise
+          })
+        })
+
+        describe('not using middleware', () => {
+          it('should taint multipart body', async () => {
+            const resultPromise = agent.assertMessageReceived(assertCmdInjection)
+
+            const formData = new FormData()
+            formData.append('command', 'echo 1')
+            await axios.post(`${proc.url}/cmd-no-middleware`, formData)
+
+            return resultPromise
+          })
+        })
+      })
+    })
+  })
+})

integration-tests/appsec/multer/body-parser-rules.json

@@ -0,0 +1,33 @@
+{
+  "version": "2.2",
+  "metadata": {
+    "rules_version": "1.5.0"
+  },
+  "rules": [
+    {
+      "id": "test-rule-id-1",
+      "name": "test-rule-name-1",
+      "tags": {
+        "type": "security_scanner",
+        "category": "attack_attempt"
+      },
+      "conditions": [
+        {
+          "parameters": {
+            "inputs": [
+              {
+                "address": "server.request.body"
+              }
+            ],
+            "list": [
+              "testattack"
+            ]
+          },
+          "operator": "phrase_match"
+        }
+      ],
+      "transformers": ["lowercase"],
+      "on_match": ["block"]
+    }
+  ]
+}

integration-tests/appsec/multer/index.js

@@ -0,0 +1,64 @@
+'use strict'
+
+const options = {
+  appsec: {
+    enabled: true
+  },
+  iast: {
+    enabled: true,
+    requestSampling: 100
+  }
+}
+
+if (process.env.AGENT_PORT) {
+  options.port = process.env.AGENT_PORT
+}
+
+if (process.env.AGENT_URL) {
+  options.url = process.env.AGENT_URL
+}
+
+const tracer = require('dd-trace')
+tracer.init(options)
+
+const http = require('http')
+const express = require('express')
+const childProcess = require('child_process')
+
+const multer = require('multer')
+const uploadToMemory = multer({ storage: multer.memoryStorage(), limits: { fileSize: 200000 } })
+
+const app = express()
+
+app.post('/', uploadToMemory.single('file'), (req, res) => {
+  res.end('DONE')
+})
+
+app.post('/no-middleware', (req, res) => {
+  uploadToMemory.none()(req, res, () => {
+    res.end('DONE')
+  })
+})
+
+app.post('/cmd', uploadToMemory.single('file'), (req, res) => {
+  childProcess.exec(req.body.command, () => {
+    res.end('DONE')
+  })
+})
+
+app.post('/cmd-no-middleware', (req, res) => {
+  uploadToMemory.none()(req, res, () => {
+    childProcess.exec(req.body.command, () => {
+      res.end('DONE')
+    })
+  })
+})
+
+app.get('/', (req, res) => {
+  res.status(200).send('hello world')
+})
+
+const server = http.createServer(app).listen(0, () => {
+  const port = server.address().port
+  process.send?.({ port })
+})

packages/datadog-instrumentations/src/helpers/hooks.js

@@ -79,6 +79,7 @@ module.exports = {
   'mongodb-core': () => require('../mongodb-core'),
   mongoose: () => require('../mongoose'),
   mquery: () => require('../mquery'),
+  multer: () => require('../multer'),
   mysql: () => require('../mysql'),
   mysql2: () => require('../mysql2'),
   net: () => require('../net'),

packages/datadog-instrumentations/src/multer.js

@@ -0,0 +1,37 @@
+'use strict'
+
+const shimmer = require('../../datadog-shimmer')
+const { channel, addHook, AsyncResource } = require('./helpers/instrument')
+
+const multerReadCh = channel('datadog:multer:read:finish')
+
+function publishRequestBodyAndNext (req, res, next) {
+  return shimmer.wrapFunction(next, next => function () {
+    if (multerReadCh.hasSubscribers && req) {
+      const abortController = new AbortController()
+      const body = req.body
+
+      multerReadCh.publish({ req, res, body, abortController })
+
+      if (abortController.signal.aborted) return
+    }
+
+    return next.apply(this, arguments)
+  })
+}
+
+addHook({
+  name: 'multer',
+  file: 'lib/make-middleware.js',
+  versions: ['^1.4.4-lts.1']
+}, makeMiddleware => {
+  return shimmer.wrapFunction(makeMiddleware, makeMiddleware => function () {
+    const middleware = makeMiddleware.apply(this, arguments)
+
+    return shimmer.wrapFunction(middleware, middleware => function wrapMulterMiddleware (req, res, next) {
+      const nextResource = new AsyncResource('bound-anonymous-fn')
+      arguments[2] = nextResource.bind(publishRequestBodyAndNext(req, res, next))
+      return middleware.apply(this, arguments)
+    })
+  })
+})