[asm] IAST security controls (#5117)

* Security controls parser and secure marks for vulnerabilities

* Use new NOSQL_MONGODB_INJECTION_MARK in nosql-injection-mongodb-analyzer

* Config

* first hooks

* wrap object properties and more tests

* Use dd-trace:moduleLoad(Start|End) channels

* iterate object strings and more tests

* fix parameter index, include createNewTainted flag and do not use PluginManager in the tests

* Fix parameter index and include a test with incorrect index

* Avoid to hook multiple times the same module and config tests

* sql_injection_mark test

* vulnerable ranges tests

* fix windows paths

* Upgrade taint-tracking to 3.3.0

* Fix * secure mark

* add createNewTainted flag to addSecureMark

* Use existing _isRangeSecure

* supressed vulnerabilities metric

* increment supressed vulnerability metric

* typo

* handle esm default export and filenames starting with file://

* esm integration tests

* clean up

* secure-marks tests

* fix secure-marks generator test

* fix config test

* empty

* check for repeated marks

* Update packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js

Co-authored-by: Ugaitz Urien <[email protected]>

* Update packages/dd-trace/src/appsec/iast/security-controls/index.js

Co-authored-by: Ugaitz Urien <[email protected]>

* Update packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js

Co-authored-by: Ugaitz Urien <[email protected]>

* some suggestions

* move _isRangeSecure to InjectionAnalyzer

* Add programatically config option

* index.d.ts

* StoredInjectionAnalyzer

* Update packages/dd-trace/test/appsec/iast/analyzers/command-injection-analyzer.spec.js

Co-authored-by: ishabi <[email protected]>

* store control keys to avoid recreating the array

* check visited before iterating

* test suggestions

* Update packages/dd-trace/src/appsec/iast/security-controls/parser.js

Co-authored-by: Ilyas Shabi <[email protected]>

* lint

* ritm test

* clean up

* Reject security control with non numeric parameters

* fix parameter 0

* Update integration-tests/appsec/iast.esm-security-controls.spec.js

Co-authored-by: Ugaitz Urien <[email protected]>

* suggestions

* use legacy store

* fix test

* fix test

* fix test

---------

Co-authored-by: Ugaitz Urien <[email protected]>
Co-authored-by: ishabi <[email protected]>

index.d.ts

@@ -2226,6 +2226,11 @@ declare namespace tracer {
      */
     redactionValuePattern?: string,
 
+    /**
+     * Allows to enable security controls.
+     */
+    securityControlsConfiguration?: string,
+
     /**
      * Specifies the verbosity of the sent telemetry. Default 'INFORMATION'
      */

integration-tests/appsec/esm-security-controls/index.mjs

@@ -0,0 +1,69 @@
+'use strict'
+
+import childProcess from 'node:child_process'
+import express from 'express'
+import { sanitize } from './sanitizer.mjs'
+import sanitizeDefault from './sanitizer-default.mjs'
+import { validate, validateNotConfigured } from './validator.mjs'
+
+const app = express()
+const port = process.env.APP_PORT || 3000
+
+app.get('/cmdi-s-secure', (req, res) => {
+  const command = sanitize(req.query.command)
+  try {
+    childProcess.execSync(command)
+  } catch (e) {
+    // ignore
+  }
+
+  res.end()
+})
+
+app.get('/cmdi-s-secure-comparison', (req, res) => {
+  const command = sanitize(req.query.command)
+  try {
+    childProcess.execSync(command)
+  } catch (e) {
+    // ignore
+  }
+
+  try {
+    childProcess.execSync(req.query.command)
+  } catch (e) {
+    // ignore
+  }
+
+  res.end()
+})
+
+app.get('/cmdi-s-secure-default', (req, res) => {
+  const command = sanitizeDefault(req.query.command)
+  try {
+    childProcess.execSync(command)
+  } catch (e) {
+    // ignore
+  }
+
+  res.end()
+})
+
+app.get('/cmdi-iv-insecure', (req, res) => {
+  if (validateNotConfigured(req.query.command)) {
+    childProcess.execSync(req.query.command)
+  }
+
+  res.end()
+})
+
+app.get('/cmdi-iv-secure', (req, res) => {
+  if (validate(req.query.command)) {
+    childProcess.execSync(req.query.command)
+  }
+
+  res.end()
+})
+
+app.listen(port, () => {
+  process.send({ port })
+})

integration-tests/appsec/esm-security-controls/sanitizer-default.mjs

@@ -0,0 +1,7 @@
+'use strict'
+
+function sanitizeDefault (input) {
+  return input
+}
+
+export default sanitizeDefault

integration-tests/appsec/esm-security-controls/sanitizer.mjs

@@ -0,0 +1,5 @@
+'use strict'
+
+export function sanitize (input) {
+  return input
+}

integration-tests/appsec/esm-security-controls/validator.mjs

@@ -0,0 +1,9 @@
+'use strict'
+
+export function validate (input) {
+  return true
+}
+
+export function validateNotConfigured (input) {
+  return true
+}

integration-tests/appsec/iast.esm-security-controls.spec.js

@@ -0,0 +1,126 @@
+'use strict'
+
+const { createSandbox, spawnProc, FakeAgent } = require('../helpers')
+const path = require('path')
+const getPort = require('get-port')
+const Axios = require('axios')
+const { assert } = require('chai')
+
+describe('ESM Security controls', () => {
+  let axios, sandbox, cwd, appPort, appFile, agent, proc
+
+  before(async function () {
+    this.timeout(process.platform === 'win32' ? 90000 : 30000)
+    sandbox = await createSandbox(['express'])
+    appPort = await getPort()
+    cwd = sandbox.folder
+    appFile = path.join(cwd, 'appsec', 'esm-security-controls', 'index.mjs')
+
+    axios = Axios.create({
+      baseURL: `http://localhost:${appPort}`
+    })
+  })
+
+  after(async function () {
+    await sandbox.remove()
+  })
+
+  const nodeOptions = '--import dd-trace/initialize.mjs'
+
+  beforeEach(async () => {
+    agent = await new FakeAgent().start()
+
+    proc = await spawnProc(appFile, {
+      cwd,
+      env: {
+        DD_TRACE_AGENT_PORT: agent.port,
+        APP_PORT: appPort,
+        DD_IAST_ENABLED: 'true',
+        DD_IAST_REQUEST_SAMPLING: '100',
+        // eslint-disable-next-line no-multi-str
+        DD_IAST_SECURITY_CONTROLS_CONFIGURATION: '\
+            SANITIZER:COMMAND_INJECTION:appsec/esm-security-controls/sanitizer.mjs:sanitize;\
+            SANITIZER:COMMAND_INJECTION:appsec/esm-security-controls/sanitizer-default.mjs;\
+            INPUT_VALIDATOR:*:appsec/esm-security-controls/validator.mjs:validate',
+        NODE_OPTIONS: nodeOptions
+      }
+    })
+  })
+
+  afterEach(async () => {
+    proc.kill()
+    await agent.stop()
+  })
+
+  it('test endpoint with iv not configured does have COMMAND_INJECTION vulnerability', async function () {
+    await axios.get('/cmdi-iv-insecure?command=ls -la')
+
+    await agent.assertMessageReceived(({ payload }) => {
+      const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
+      spans.forEach(span => {
+        assert.property(span.meta, '_dd.iast.json')
+        assert.include(span.meta['_dd.iast.json'], '"COMMAND_INJECTION"')
+      })
+    }, null, 1, true)
+  })
+
+  it('test endpoint sanitizer does not have COMMAND_INJECTION vulnerability', async () => {
+    await axios.get('/cmdi-s-secure?command=ls -la')
+
+    await agent.assertMessageReceived(({ payload }) => {
+      const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
+      spans.forEach(span => {
+        assert.notProperty(span.meta, '_dd.iast.json')
+        assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
+      })
+    }, null, 1, true)
+  })
+
+  it('test endpoint with default sanitizer does not have COMMAND_INJECTION vulnerability', async () => {
+    await axios.get('/cmdi-s-secure-default?command=ls -la')
+
+    await agent.assertMessageReceived(({ payload }) => {
+      const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
+      spans.forEach(span => {
+        assert.notProperty(span.meta, '_dd.iast.json')
+        assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
+      })
+    }, null, 1, true)
+  })
+
+  it('test endpoint with default sanitizer does have COMMAND_INJECTION with original tainted', async () => {
+    await axios.get('/cmdi-s-secure-comparison?command=ls -la')
+
+    await agent.assertMessageReceived(({ payload }) => {
+      const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
+      spans.forEach(span => {
+        assert.property(span.meta, '_dd.iast.json')
+        assert.include(span.meta['_dd.iast.json'], '"COMMAND_INJECTION"')
+      })
+    }, null, 1, true)
+  })
+
+  it('test endpoint with default sanitizer does have COMMAND_INJECTION vulnerability', async () => {
+    await axios.get('/cmdi-s-secure-default?command=ls -la')
+
+    await agent.assertMessageReceived(({ payload }) => {
+      const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
+      spans.forEach(span => {
+        assert.notProperty(span.meta, '_dd.iast.json')
+        assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
+      })
+    }, null, 1, true)
+  })
+
+  it('test endpoint with iv does not have COMMAND_INJECTION vulnerability', async () => {
+    await axios.get('/cmdi-iv-secure?command=ls -la')
+
+    await agent.assertMessageReceived(({ payload }) => {
+      const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request'))
+      spans.forEach(span => {
+        assert.notProperty(span.meta, '_dd.iast.json')
+        assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection')
+      })
+    }, null, 1, true)
+  })
+})

package.json

@@ -84,7 +84,7 @@
     "@datadog/libdatadog": "^0.4.0",
     "@datadog/native-appsec": "8.4.0",
     "@datadog/native-iast-rewriter": "2.8.0",
-    "@datadog/native-iast-taint-tracking": "3.2.0",
+    "@datadog/native-iast-taint-tracking": "3.3.0",
     "@datadog/native-metrics": "^3.1.0",
     "@datadog/pprof": "5.5.1",
     "@datadog/sketches-js": "^2.1.0",

packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js

@@ -1,12 +1,12 @@
 'use strict'
 
-const InjectionAnalyzer = require('./injection-analyzer')
 const { CODE_INJECTION } = require('../vulnerabilities')
+const StoredInjectionAnalyzer = require('./stored-injection-analyzer')
 const { INSTRUMENTED_SINK } = require('../telemetry/iast-metric')
 const { storage } = require('../../../../../datadog-core')
 const { getIastContext } = require('../iast-context')
 
-class CodeInjectionAnalyzer extends InjectionAnalyzer {
+class CodeInjectionAnalyzer extends StoredInjectionAnalyzer {
   constructor () {
     super(CODE_INJECTION)
     this.evalInstrumentedInc = false
@@ -31,10 +31,6 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer {
     this.addSub('datadog:vm:run-script:start', ({ code }) => this.analyze(code))
     this.addSub('datadog:vm:source-text-module:start', ({ code }) => this.analyze(code))
   }
-
-  _areRangesVulnerable () {
-    return true
-  }
 }
 
 module.exports = new CodeInjectionAnalyzer()

packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js

@@ -5,8 +5,13 @@ const { SQL_ROW_VALUE } = require('../taint-tracking/source-types')
 
 class InjectionAnalyzer extends Analyzer {
   _isVulnerable (value, iastContext) {
-    const ranges = value && getRanges(iastContext, value)
+    let ranges = value && getRanges(iastContext, value)
     if (ranges?.length > 0) {
+      ranges = this._filterSecureRanges(ranges)
+      if (!ranges?.length) {
+        this._incrementSuppressedMetric(iastContext)
+      }
+
       return this._areRangesVulnerable(ranges)
     }
 
@@ -21,6 +26,15 @@ class InjectionAnalyzer extends Analyzer {
   _areRangesVulnerable (ranges) {
     return ranges?.some(range => range.iinfo.type !== SQL_ROW_VALUE)
   }
+
+  _filterSecureRanges (ranges) {
+    return ranges?.filter(range => !this._isRangeSecure(range))
+  }
+
+  _isRangeSecure (range) {
+    const { secureMarks } = range
+    return (secureMarks & this._secureMark) === this._secureMark
+  }
 }
 
 module.exports = InjectionAnalyzer

packages/dd-trace/src/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.js

@@ -4,29 +4,13 @@ const InjectionAnalyzer = require('./injection-analyzer')
 const { NOSQL_MONGODB_INJECTION } = require('../vulnerabilities')
 const { getRanges, addSecureMark } = require('../taint-tracking/operations')
 const { getNodeModulesPaths } = require('../path-line')
-const { getNextSecureMark } = require('../taint-tracking/secure-marks-generator')
 const { storage } = require('../../../../../datadog-core')
 const { getIastContext } = require('../iast-context')
 const { HTTP_REQUEST_PARAMETER, HTTP_REQUEST_BODY } = require('../taint-tracking/source-types')
 
 const EXCLUDED_PATHS_FROM_STACK = getNodeModulesPaths('mongodb', 'mongoose', 'mquery')
-const MONGODB_NOSQL_SECURE_MARK = getNextSecureMark()
-
-function iterateObjectStrings (target, fn, levelKeys = [], depth = 20, visited = new Set()) {
-  if (target !== null && typeof target === 'object') {
-    Object.keys(target).forEach((key) => {
-      const nextLevelKeys = [...levelKeys, key]
-      const val = target[key]
-
-      if (typeof val === 'string') {
-        fn(val, nextLevelKeys, target, key)
-      } else if (depth > 0 && !visited.has(val)) {
-        iterateObjectStrings(val, fn, nextLevelKeys, depth - 1, visited)
-        visited.add(val)
-      }
-    })
-  }
-}
+const { NOSQL_MONGODB_INJECTION_MARK } = require('../taint-tracking/secure-marks')
+const { iterateObjectStrings } = require('../utils')
 
 class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
   constructor () {
@@ -88,7 +72,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
                 const currentLevelKey = levelKeys[i]
 
                 if (i === levelsLength - 1) {
-                  parentObj[currentLevelKey] = addSecureMark(iastContext, value, MONGODB_NOSQL_SECURE_MARK)
+                  parentObj[currentLevelKey] = addSecureMark(iastContext, value, NOSQL_MONGODB_INJECTION_MARK)
                 } else {
                   parentObj = parentObj[currentLevelKey]
                 }
@@ -106,7 +90,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
       if (iastContext) { // do nothing if we are not in an iast request
         iterateObjectStrings(sanitizedObject, function (value, levelKeys, parent, lastKey) {
           try {
-            parent[lastKey] = addSecureMark(iastContext, value, MONGODB_NOSQL_SECURE_MARK)
+            parent[lastKey] = addSecureMark(iastContext, value, NOSQL_MONGODB_INJECTION_MARK)
           } catch {
             // if it is a readonly property, do nothing
           }
@@ -121,8 +105,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
 
   _isVulnerableRange (range) {
     const rangeType = range?.iinfo?.type
-    const isVulnerableType = rangeType === HTTP_REQUEST_PARAMETER || rangeType === HTTP_REQUEST_BODY
-    return isVulnerableType && (range.secureMarks & MONGODB_NOSQL_SECURE_MARK) !== MONGODB_NOSQL_SECURE_MARK
+    return rangeType === HTTP_REQUEST_PARAMETER || rangeType === HTTP_REQUEST_BODY
   }
 
   _isVulnerable (value, iastContext) {
@@ -137,10 +120,15 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
       const allRanges = []
 
       iterateObjectStrings(value.filter, (val, nextLevelKeys) => {
-        const ranges = getRanges(iastContext, val)
+        let ranges = getRanges(iastContext, val)
         if (ranges?.length) {
           const filteredRanges = []
 
+          ranges = this._filterSecureRanges(ranges)
+          if (!ranges.length) {
+            this._incrementSuppressedMetric(iastContext)
+          }
+
           for (const range of ranges) {
             if (this._isVulnerableRange(range)) {
               isVulnerable = true
@@ -175,4 +163,3 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer {
 }
 
 module.exports = new NosqlInjectionMongodbAnalyzer()
-module.exports.MONGODB_NOSQL_SECURE_MARK = MONGODB_NOSQL_SECURE_MARK