Exploit Prevention LFI

packages/dd-trace/src/appsec/channels.js

@@ -26,5 +26,6 @@ module.exports = {
   pgPoolQueryStart: dc.channel('datadog:pg:pool:query:start'),
   mysql2OuterQueryStart: dc.channel('datadog:mysql2:outerquery:start'),
   wafRunFinished: dc.channel('datadog:waf:run:finish'),
-  fsOperationStart: dc.channel('apm:fs:operation:start')
+  fsOperationStart: dc.channel('apm:fs:operation:start'),
+  expressMiddlewareError: dc.channel('apm:express:middleware:error')
 }

packages/dd-trace/src/appsec/rasp/index.js

@@ -1,8 +1,8 @@
 'use strict'
 
 const web = require('../../plugins/util/web')
-const { setUncaughtExceptionCaptureCallbackStart } = require('../channels')
-const { block } = require('../blocking')
+const { setUncaughtExceptionCaptureCallbackStart, expressMiddlewareError } = require('../channels')
+const { block, isBlocked } = require('../blocking')
 const ssrf = require('./ssrf')
 const sqli = require('./sql_injection')
 const lfi = require('./lfi')
@@ -31,17 +31,13 @@ function findDatadogRaspAbortError (err, deep = 10) {
     return err
   }
 
-  if (err.cause && deep > 0) {
+  if (err?.cause && deep > 0) {
     return findDatadogRaspAbortError(err.cause, deep - 1)
   }
 }
 
-function handleUncaughtExceptionMonitor (err) {
-  const abortError = findDatadogRaspAbortError(err)
-  if (!abortError) return
-
-  const { req, res, blockingAction } = abortError
-  block(req, res, web.root(req), null, blockingAction)
+function handleUncaughtExceptionMonitor (error) {
+  if (!blockOnDatadogRaspAbortError({ error })) return
 
   if (!process.hasUncaughtExceptionCaptureCallback()) {
     const cleanUp = removeAllListeners(process, 'uncaughtException')
@@ -83,12 +79,25 @@ function handleUncaughtExceptionMonitor (err) {
   }
 }
 
+function blockOnDatadogRaspAbortError ({ error }) {
+  const abortError = findDatadogRaspAbortError(error)
+  if (!abortError) return false
+
+  const { req, res, blockingAction } = abortError
+  if (!isBlocked(res)) {
+    block(req, res, web.root(req), null, blockingAction)
+  }
+
+  return true
+}
+
 function enable (config) {
   ssrf.enable(config)
   sqli.enable(config)
   lfi.enable(config)
 
   process.on('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor)
+  expressMiddlewareError.subscribe(blockOnDatadogRaspAbortError)
 }
 
 function disable () {
@@ -97,10 +106,12 @@ function disable () {
   lfi.disable()
 
   process.off('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor)
+  if (expressMiddlewareError.hasSubscribers) expressMiddlewareError.unsubscribe(blockOnDatadogRaspAbortError)
 }
 
 module.exports = {
   enable,
   disable,
-  handleUncaughtExceptionMonitor // exported only for testing purpose
+  handleUncaughtExceptionMonitor, // exported only for testing purpose
+  blockOnDatadogRaspAbortError // exported only for testing purpose
 }

packages/dd-trace/test/appsec/rasp/index.spec.js

@@ -1,9 +1,14 @@
 'use strict'
 
+const proxyquire = require('proxyquire')
+const { expressMiddlewareError } = require('../../../src/appsec/channels')
 const rasp = require('../../../src/appsec/rasp')
 const { handleUncaughtExceptionMonitor } = require('../../../src/appsec/rasp')
+const { DatadogRaspAbortError } = require('../../../src/appsec/rasp/utils')
 
 describe('RASP', () => {
+  let onErrorSub, onErrorUnsub, block
+
   beforeEach(() => {
     const config = {
       appsec: {
@@ -15,6 +20,9 @@ describe('RASP', () => {
       }
     }
 
+    onErrorSub = sinon.spy(expressMiddlewareError, 'subscribe')
+    onErrorUnsub = sinon.spy(expressMiddlewareError, 'unsubscribe')
+
     rasp.enable(config)
   })
 
@@ -31,4 +39,61 @@ describe('RASP', () => {
       handleUncaughtExceptionMonitor(err)
     })
   })
+
+  describe('enable/disable', () => {
+    it('should subscribe to apm:express:middleware:error', () => {
+      sinon.assert.calledOnce(onErrorSub)
+    })
+
+    it('should unsubscribe to apm:express:middleware:error', () => {
+      rasp.disable()
+
+      sinon.assert.calledOnce(onErrorUnsub)
+    })
+  })
+
+  describe('blockOnDatadogRaspAbortError', () => {
+    let rasp, req, res, blockingAction, blocked
+
+    beforeEach(() => {
+      req = {}
+      res = {}
+      blockingAction = 'block'
+      block = sinon.stub()
+
+      rasp = proxyquire('../../../src/appsec/rasp', {
+        '../blocking': {
+          block,
+          isBlocked: sinon.stub().callsFake(() => blocked)
+        }
+      })
+    })
+
+    afterEach(() => {
+      sinon.restore()
+    })
+
+    it('should skip non DatadogRaspAbortError', () => {
+      rasp.blockOnDatadogRaspAbortError({ error: new Error() })
+
+      sinon.assert.notCalled(block)
+    })
+
+    it('should block DatadogRaspAbortError first time', () => {
+      rasp.blockOnDatadogRaspAbortError({ error: new DatadogRaspAbortError(req, res, blockingAction) })
+
+      sinon.assert.calledOnce(block)
+    })
+
+    it('should skip calling block if blocked before', () => {
+      rasp.blockOnDatadogRaspAbortError({ error: new DatadogRaspAbortError(req, res, blockingAction) })
+
+      blocked = true
+
+      rasp.blockOnDatadogRaspAbortError({ error: new DatadogRaspAbortError(req, res, blockingAction) })
+      rasp.blockOnDatadogRaspAbortError({ error: new DatadogRaspAbortError(req, res, blockingAction) })
+
+      sinon.assert.calledOnce(block)
+    })
+  })
 })

packages/dd-trace/test/appsec/rasp/lfi.express.plugin.spec.js

@@ -82,12 +82,28 @@ describe('RASP - lfi', () => {
         }
       }
 
+      function getAppSync (fn, args, options) {
+        return (req, res) => {
+          try {
+            const result = fn(args)
+            options.onfinish?.(result)
+          } catch (e) {
+            if (e.message === 'DatadogRaspAbortError') {
+              res.writeHead(418)
+            }
+          }
+          res.end('end')
+        }
+      }
+
       function runFsMethodTest (description, options, fn, ...args) {
         const { vulnerableIndex = 0, ruleEvalCount } = options
 
         describe(description, () => {
+          const getAppFn = options.getAppFn ?? getApp
+
           it('should block param from the request', async () => {
-            app = getApp(fn, args, options)
+            app = getAppFn(fn, args, options)
 
             const file = args[vulnerableIndex]
             return testBlockingRequest(`/?file=${file}`, undefined, ruleEvalCount)
@@ -97,7 +113,7 @@ describe('RASP - lfi', () => {
           })
 
           it('should not block if param not found in the request', async () => {
-            app = getApp(fn, args, options)
+            app = getAppFn(fn, args, options)
 
             await axios.get('/?file=/test.file')
 
@@ -113,14 +129,8 @@ describe('RASP - lfi', () => {
           desc += ` with vulnerable index ${vulnerableIndex}`
         }
         describe(desc, () => {
-          runFsMethodTest(`test fs.${methodName}Sync method`, options, (args) => {
-            return new Promise((resolve, reject) => {
-              try {
-                resolve(require('fs')[`${methodName}Sync`](...args))
-              } catch (e) {
-                reject(e)
-              }
-            })
+          runFsMethodTest(`test fs.${methodName}Sync method`, { ...options, getAppFn: getAppSync }, (args) => {
+            return require('fs')[`${methodName}Sync`](...args)
           }, ...args)
 
           runFsMethodTest(`test fs.${methodName} method`, options, (args) => {