Exploit prevention command injection (#4966)

* Exploit prevention command injection * fix spawnSync abort error test * add telemetry tests * fix sql injection tests on postgres * add different test * revert spawnSync changes * fix linter * add spawnSync tests * remove spawnSync not needed test * fix cmdi params * Revert "fix cmdi params" This reverts commit 4a3d766.
DataDog · Dec 19, 2024 · 4d5962d · 4d5962d
1 parent 6b7cab6
commit 4d5962d
Show file tree

Hide file tree

Showing 22 changed files with 328 additions and 155 deletions.
diff --git a/packages/dd-trace/src/appsec/addresses.js b/packages/dd-trace/src/appsec/addresses.js
@@ -31,6 +31,7 @@ module.exports = {
   DB_STATEMENT: 'server.db.statement',
   DB_SYSTEM: 'server.db.system',
 
+  EXEC_COMMAND: 'server.sys.exec.cmd',
   SHELL_COMMAND: 'server.sys.shell.cmd',
 
   LOGIN_SUCCESS: 'server.business_logic.users.login.success',

diff --git a/packages/dd-trace/src/appsec/rasp/command_injection.js b/packages/dd-trace/src/appsec/rasp/command_injection.js
@@ -25,19 +25,26 @@ function disable () {
 }
 
 function analyzeCommandInjection ({ file, fileArgs, shell, abortController }) {
-  if (!file || !shell) return
+  if (!file) return
 
   const store = storage.getStore()
   const req = store?.req
   if (!req) return
 
-  const commandParams = fileArgs ? [file, ...fileArgs] : file
-
-  const persistent = {
-    [addresses.SHELL_COMMAND]: commandParams
+  const persistent = {}
+  const raspRule = { type: RULE_TYPES.COMMAND_INJECTION }
+  const params = fileArgs ? [file, ...fileArgs] : file
+
+  if (shell) {
+    persistent[addresses.SHELL_COMMAND] = params
+    raspRule.variant = 'shell'
+  } else {
+    const commandParams = Array.isArray(params) ? params : [params]
+    persistent[addresses.EXEC_COMMAND] = commandParams
+    raspRule.variant = 'exec'
   }
 
-  const result = waf.run({ persistent }, req, RULE_TYPES.COMMAND_INJECTION)
+  const result = waf.run({ persistent }, req, raspRule)
 
   const res = store?.res
   handleResult(result, req, res, abortController, config)

diff --git a/packages/dd-trace/src/appsec/rasp/lfi.js b/packages/dd-trace/src/appsec/rasp/lfi.js
@@ -58,7 +58,9 @@ function analyzeLfi (ctx) {
       [FS_OPERATION_PATH]: path
     }
 
-    const result = waf.run({ persistent }, req, RULE_TYPES.LFI)
+    const raspRule = { type: RULE_TYPES.LFI }
+
+    const result = waf.run({ persistent }, req, raspRule)
     handleResult(result, req, res, ctx.abortController, config)
   })
 }

diff --git a/packages/dd-trace/src/appsec/rasp/sql_injection.js b/packages/dd-trace/src/appsec/rasp/sql_injection.js
@@ -72,7 +72,9 @@ function analyzeSqlInjection (query, dbSystem, abortController) {
     [addresses.DB_SYSTEM]: dbSystem
   }
 
-  const result = waf.run({ persistent }, req, RULE_TYPES.SQL_INJECTION)
+  const raspRule = { type: RULE_TYPES.SQL_INJECTION }
+
+  const result = waf.run({ persistent }, req, raspRule)
 
   handleResult(result, req, res, abortController, config)
 }

diff --git a/packages/dd-trace/src/appsec/rasp/ssrf.js b/packages/dd-trace/src/appsec/rasp/ssrf.js
@@ -29,7 +29,9 @@ function analyzeSsrf (ctx) {
     [addresses.HTTP_OUTGOING_URL]: outgoingUrl
   }
 
-  const result = waf.run({ persistent }, req, RULE_TYPES.SSRF)
+  const raspRule = { type: RULE_TYPES.SSRF }
+
+  const result = waf.run({ persistent }, req, raspRule)
 
   const res = store?.res
   handleResult(result, req, res, ctx.abortController, config)

diff --git a/packages/dd-trace/src/appsec/remote_config/capabilities.js b/packages/dd-trace/src/appsec/remote_config/capabilities.js
@@ -25,5 +25,6 @@ module.exports = {
   ASM_AUTO_USER_INSTRUM_MODE: 1n << 31n,
   ASM_ENDPOINT_FINGERPRINT: 1n << 32n,
   ASM_NETWORK_FINGERPRINT: 1n << 34n,
-  ASM_HEADER_FINGERPRINT: 1n << 35n
+  ASM_HEADER_FINGERPRINT: 1n << 35n,
+  ASM_RASP_CMDI: 1n << 37n
 }
diff --git a/packages/dd-trace/src/appsec/remote_config/index.js b/packages/dd-trace/src/appsec/remote_config/index.js
@@ -101,6 +101,7 @@ function enableWafUpdate (appsecConfig) {
       rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_SSRF, true)
       rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_LFI, true)
       rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_SHI, true)
+      rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_CMDI, true)
     }
 
     // TODO: delete noop handlers and kPreUpdate and replace with batched handlers
@@ -133,6 +134,7 @@ function disableWafUpdate () {
     rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_SSRF, false)
     rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_LFI, false)
     rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_SHI, false)
+    rc.updateCapabilities(RemoteConfigCapabilities.ASM_RASP_CMDI, false)
 
     rc.removeProductHandler('ASM_DATA')
     rc.removeProductHandler('ASM_DD')

diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js
@@ -101,16 +101,16 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}) {
   incrementWafInitMetric(wafVersion, rulesVersion)
 }
 
-function reportMetrics (metrics, raspRuleType) {
+function reportMetrics (metrics, raspRule) {
   const store = storage.getStore()
   const rootSpan = store?.req && web.root(store.req)
   if (!rootSpan) return
 
   if (metrics.rulesVersion) {
     rootSpan.setTag('_dd.appsec.event_rules.version', metrics.rulesVersion)
   }
-  if (raspRuleType) {
-    updateRaspRequestsMetricTags(metrics, store.req, raspRuleType)
+  if (raspRule) {
+    updateRaspRequestsMetricTags(metrics, store.req, raspRule)
   } else {
     updateWafRequestsMetricTags(metrics, store.req)
   }

diff --git a/packages/dd-trace/src/appsec/telemetry.js b/packages/dd-trace/src/appsec/telemetry.js
@@ -79,7 +79,7 @@ function getOrCreateMetricTags (store, versionsTags) {
   return metricTags
 }
 
-function updateRaspRequestsMetricTags (metrics, req, raspRuleType) {
+function updateRaspRequestsMetricTags (metrics, req, raspRule) {
   if (!req) return
 
   const store = getStore(req)
@@ -89,7 +89,12 @@ function updateRaspRequestsMetricTags (metrics, req, raspRuleType) {
 
   if (!enabled) return
 
-  const tags = { rule_type: raspRuleType, waf_version: metrics.wafVersion }
+  const tags = { rule_type: raspRule.type, waf_version: metrics.wafVersion }
+
+  if (raspRule.variant) {
+    tags.rule_variant = raspRule.variant
+  }
+
   appsecMetrics.count('rasp.rule.eval', tags).inc(1)
 
   if (metrics.wafTimeout) {

diff --git a/packages/dd-trace/src/appsec/waf/index.js b/packages/dd-trace/src/appsec/waf/index.js
@@ -46,7 +46,7 @@ function update (newRules) {
   }
 }
 
-function run (data, req, raspRuleType) {
+function run (data, req, raspRule) {
   if (!req) {
     const store = storage.getStore()
     if (!store || !store.req) {
@@ -59,7 +59,7 @@ function run (data, req, raspRuleType) {
 
   const wafContext = waf.wafManager.getWAFContext(req)
 
-  return wafContext.run(data, raspRuleType)
+  return wafContext.run(data, raspRule)
 }
 
 function disposeContext (req) {

diff --git a/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js b/packages/dd-trace/src/appsec/waf/waf_context_wrapper.js
@@ -21,7 +21,7 @@ class WAFContextWrapper {
     this.knownAddresses = knownAddresses
   }
 
-  run ({ persistent, ephemeral }, raspRuleType) {
+  run ({ persistent, ephemeral }, raspRule) {
     if (this.ddwafContext.disposed) {
       log.warn('[ASM] Calling run on a disposed context')
       return
@@ -87,7 +87,7 @@ class WAFContextWrapper {
         blockTriggered,
         wafVersion: this.wafVersion,
         wafTimeout: result.timeout
-      }, raspRuleType)
+      }, raspRule)
 
       if (ruleTriggered) {
         Reporter.reportAttack(JSON.stringify(result.events))