Fixes

DataDog · Oct 22, 2024 · b039787 · b039787
1 parent 97fd1c8
commit b039787
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 20 deletions.
diff --git a/packages/datadog-instrumentations/src/child_process.js b/packages/datadog-instrumentations/src/child_process.js
@@ -23,7 +23,6 @@ names.forEach(name => {
     if (!patched) {
       patched = true
       shimmer.massWrap(childProcess, execAsyncMethods, wrapChildProcessAsyncMethod(childProcess.ChildProcess))
-      // shimmer.massWrap(childProcess, execSyncMethods, wrapChildProcessSyncMethod())
       shimmer.wrap(childProcess, 'execSync', wrapChildProcessSyncMethod('execSync', true))
       shimmer.wrap(childProcess, 'execFileSync', wrapChildProcessSyncMethod('execFileSync'))
       shimmer.wrap(childProcess, 'spawnSync', wrapChildProcessSyncMethod('spawnSync'))
@@ -83,11 +82,12 @@ function wrapChildProcessSyncMethod (methodName, shell = false) {
         try {
           if (abortController.signal.aborted) {
             const error = abortController.signal.reason || new Error('Aborted')
-            // expected results on error are different in each method
+            // expected behaviors on error are different
             switch (methodName) {
               case 'execFileSync':
               case 'execSync':
                 throw error
+
               case 'spawnSync':
                 context.result = {
                   error,
@@ -146,7 +146,7 @@ function wrapChildProcessCustomPromisifyMethod (customPromisifyMethod, shell) {
     })
 
     let result
-    if (abortController) {
+    if (abortController.signal.aborted) {
       result = Promise.reject(abortController.signal.reason || new Error('Aborted'))
     } else {
       try {

diff --git a/packages/datadog-instrumentations/test/child_process.spec.js b/packages/datadog-instrumentations/test/child_process.spec.js
@@ -6,11 +6,6 @@ const dc = require('dc-polyfill')
 const { NODE_MAJOR } = require('../../../version')
 
 describe('child process', () => {
-  // const modules = ['child_process']
-  // const execAsyncMethods = ['execFile']
-  // const execAsyncShellMethods = []
-  // const execSyncMethods = []
-  // const execSyncShellMethods = []
   const modules = ['child_process', 'node:child_process']
   const execAsyncMethods = ['execFile', 'spawn']
   const execAsyncShellMethods = ['exec']

diff --git a/packages/dd-trace/test/appsec/rasp/command_injection.spec.js b/packages/dd-trace/test/appsec/rasp/command_injection.spec.js
@@ -1,8 +1,10 @@
 'use strict'
 
 const proxyquire = require('proxyquire')
-const { childProcessExecutionStart } = require('../../../src/appsec/channels')
 const addresses = require('../../../src/appsec/addresses')
+const { childProcessExecutionTracingChannel } = require('../../../src/appsec/channels')
+
+const { start } = childProcessExecutionTracingChannel
 
 describe('RASP - command_injection.js', () => {
   let waf, datadogCore, commandInjection, utils, config
@@ -49,12 +51,13 @@ describe('RASP - command_injection.js', () => {
   describe('analyzeCommandInjection', () => {
     it('should analyze command_injection without arguments', () => {
       const ctx = {
-        file: 'cmd'
+        file: 'cmd',
+        shell: true
       }
       const req = {}
       datadogCore.storage.getStore.returns({ req })
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
       const persistent = { [addresses.SHELL_COMMAND]: 'cmd' }
       sinon.assert.calledOnceWithExactly(waf.run, { persistent }, req, 'command_injection')
@@ -63,17 +66,33 @@ describe('RASP - command_injection.js', () => {
     it('should analyze command_injection with arguments', () => {
       const ctx = {
         file: 'cmd',
-        fileArgs: ['arg0', 'arg1']
+        fileArgs: ['arg0', 'arg1'],
+        shell: true
       }
       const req = {}
       datadogCore.storage.getStore.returns({ req })
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
-      const persistent = { [addresses.SHELL_COMMAND]: ['cmd', 'arg0', 'arg1'] }
+      // TODO remove join with new libddwaf version
+      const persistent = { [addresses.SHELL_COMMAND]: ['cmd', 'arg0', 'arg1'].join(' ') }
       sinon.assert.calledOnceWithExactly(waf.run, { persistent }, req, 'command_injection')
     })
 
+    it('should not analyze command_injection when it is not shell', () => {
+      const ctx = {
+        file: 'cmd',
+        fileArgs: ['arg0', 'arg1'],
+        shell: false
+      }
+      const req = {}
+      datadogCore.storage.getStore.returns({ req })
+
+      start.publish(ctx)
+
+      sinon.assert.notCalled(waf.run)
+    })
+
     it('should not analyze command_injection if rasp is disabled', () => {
       commandInjection.disable()
       const ctx = {
@@ -82,7 +101,7 @@ describe('RASP - command_injection.js', () => {
       const req = {}
       datadogCore.storage.getStore.returns({ req })
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
       sinon.assert.notCalled(waf.run)
     })
@@ -93,7 +112,7 @@ describe('RASP - command_injection.js', () => {
       }
       datadogCore.storage.getStore.returns(undefined)
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
       sinon.assert.notCalled(waf.run)
     })
@@ -104,7 +123,7 @@ describe('RASP - command_injection.js', () => {
       }
       datadogCore.storage.getStore.returns({})
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
       sinon.assert.notCalled(waf.run)
     })
@@ -115,21 +134,21 @@ describe('RASP - command_injection.js', () => {
       }
       datadogCore.storage.getStore.returns({})
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
       sinon.assert.notCalled(waf.run)
     })
 
     it('should call handleResult', () => {
       const abortController = { abort: 'abort' }
-      const ctx = { file: 'cmd', abortController }
+      const ctx = { file: 'cmd', abortController, shell: true }
       const wafResult = { waf: 'waf' }
       const req = { req: 'req' }
       const res = { res: 'res' }
       waf.run.returns(wafResult)
       datadogCore.storage.getStore.returns({ req, res })
 
-      childProcessExecutionStart.publish(ctx)
+      start.publish(ctx)
 
       sinon.assert.calledOnceWithExactly(utils.handleResult, wafResult, req, res, abortController, config)
     })