diff --git a/packages/datadog-instrumentations/src/child_process.js b/packages/datadog-instrumentations/src/child_process.js index 8af4978800..342e1ba284 100644 --- a/packages/datadog-instrumentations/src/child_process.js +++ b/packages/datadog-instrumentations/src/child_process.js @@ -13,7 +13,6 @@ const childProcessChannel = dc.tracingChannel('datadog:child_process:execution') // ignored exec method because it calls to execFile directly const execAsyncMethods = ['execFile', 'spawn'] -const execSyncMethods = ['execFileSync', 'spawnSync'] const names = ['child_process', 'node:child_process'] @@ -23,9 +22,11 @@ names.forEach(name => { addHook({ name }, childProcess => { if (!patched) { patched = true - shimmer.massWrap(childProcess, execAsyncMethods, wrapChildProcessAsyncMethod()) - shimmer.massWrap(childProcess, execSyncMethods, wrapChildProcessSyncMethod()) - shimmer.wrap(childProcess, 'execSync', wrapChildProcessSyncMethod(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')) } return childProcess @@ -34,17 +35,21 @@ names.forEach(name => { function normalizeArgs (args, shell) { const childProcessInfo = { - command: args[0] + command: args[0], + file: args[0] } if (Array.isArray(args[1])) { childProcessInfo.command = childProcessInfo.command + ' ' + args[1].join(' ') + childProcessInfo.fileArgs = args[1] + if (args[2] !== null && typeof args[2] === 'object') { childProcessInfo.options = args[2] } } else if (args[1] !== null && typeof args[1] === 'object') { childProcessInfo.options = args[1] } + childProcessInfo.shell = shell || childProcessInfo.options?.shell === true || typeof childProcessInfo.options?.shell === 'string' @@ -52,7 +57,7 @@ function normalizeArgs (args, shell) { return childProcessInfo } -function wrapChildProcessSyncMethod (shell = false) { +function wrapChildProcessSyncMethod (methodName, shell = false) { return function wrapMethod (childProcessMethod) { return function () { if (!childProcessChannel.start.hasSubscribers || arguments.length === 0) { @@ -63,14 +68,53 @@ function wrapChildProcessSyncMethod (shell = false) { const innerResource = new AsyncResource('bound-anonymous-fn') return innerResource.runInAsyncScope(() => { - return childProcessChannel.traceSync( - childProcessMethod, - { - command: childProcessInfo.command, - shell: childProcessInfo.shell - }, - this, - ...arguments) + const context = { + command: childProcessInfo.command, + file: childProcessInfo.file, + shell: childProcessInfo.shell + } + if (childProcessInfo.fileArgs) { + context.fileArgs = childProcessInfo.fileArgs + } + const abortController = new AbortController() + + childProcessChannel.start.publish({ ...context, abortController }) + + try { + if (abortController.signal.aborted) { + const error = abortController.signal.reason || new Error('Aborted') + // expected results on error are different in each method + switch (methodName) { + case 'execFileSync': + case 'execSync': + throw error + case 'spawnSync': + context.result = { + error, + status: null, + signal: null, + output: null, + stdout: null, + stderr: null, + pid: 0 + } + + return context.result + } + } + + const result = childProcessMethod.apply(this, arguments) + context.result = result + + return result + } catch (err) { + context.error = err + childProcessChannel.error.publish(context) + + throw err + } finally { + childProcessChannel.end.publish(context) + } }) } } @@ -84,18 +128,59 @@ function wrapChildProcessCustomPromisifyMethod (customPromisifyMethod, shell) { const childProcessInfo = normalizeArgs(arguments, shell) - return childProcessChannel.tracePromise( - customPromisifyMethod, - { - command: childProcessInfo.command, - shell: childProcessInfo.shell - }, - this, - ...arguments) + const context = { + command: childProcessInfo.command, + file: childProcessInfo.file, + shell: childProcessInfo.shell + } + if (childProcessInfo.fileArgs) { + context.fileArgs = childProcessInfo.fileArgs + } + + const { start, end, asyncStart, asyncEnd, error } = childProcessChannel + const abortController = new AbortController() + + start.publish({ + ...context, + abortController + }) + + let result + if (abortController) { + result = Promise.reject(abortController.signal.reason || new Error('Aborted')) + } else { + try { + result = customPromisifyMethod.apply(this, arguments) + } catch (error) { + error.publish({ ...context, error }) + throw error + } finally { + end.publish(context) + } + } + + function reject (err) { + context.error = err + error.publish(context) + asyncStart.publish(context) + + asyncEnd.publish(context) + return Promise.reject(err) + } + + function resolve (result) { + context.result = result + asyncStart.publish(context) + + asyncEnd.publish(context) + return result + } + + return Promise.prototype.then.call(result, resolve, reject) } } -function wrapChildProcessAsyncMethod (shell = false) { +function wrapChildProcessAsyncMethod (ChildProcess, shell = false) { return function wrapMethod (childProcessMethod) { function wrappedChildProcessMethod () { if (!childProcessChannel.start.hasSubscribers || arguments.length === 0) { @@ -112,9 +197,39 @@ function wrapChildProcessAsyncMethod (shell = false) { const innerResource = new AsyncResource('bound-anonymous-fn') return innerResource.runInAsyncScope(() => { - childProcessChannel.start.publish({ command: childProcessInfo.command, shell: childProcessInfo.shell }) + const abortController = new AbortController() + const { command, file, shell, fileArgs } = childProcessInfo + const context = { + command, + file, + shell + } + if (fileArgs) { + context.fileArgs = fileArgs + } + + childProcessChannel.start.publish({ ...context, abortController }) + + let childProcess + if (abortController.signal.aborted) { + childProcess = new ChildProcess() + childProcess.on('error', () => {}) // Original method does not crash when non subscribers + + process.nextTick(() => { + const error = abortController.signal.reason || new Error('Aborted') + childProcess.emit('error', error) + + const cb = arguments[arguments.length - 1] + if (typeof cb === 'function') { + cb(error) + } + + childProcess.emit('close') + }) + } else { + childProcess = childProcessMethod.apply(this, arguments) + } - const childProcess = childProcessMethod.apply(this, arguments) if (childProcess) { let errorExecuted = false @@ -129,8 +244,7 @@ function wrapChildProcessAsyncMethod (shell = false) { childProcessChannel.error.publish() } childProcessChannel.asyncEnd.publish({ - command: childProcessInfo.command, - shell: childProcessInfo.shell, + ...context, result: code }) }) diff --git a/packages/datadog-instrumentations/test/child_process.spec.js b/packages/datadog-instrumentations/test/child_process.spec.js index ffd002e8a6..661adab202 100644 --- a/packages/datadog-instrumentations/test/child_process.spec.js +++ b/packages/datadog-instrumentations/test/child_process.spec.js @@ -6,10 +6,15 @@ 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'] - const execSyncMethods = ['execFileSync'] + const execSyncMethods = ['execFileSync', 'spawnSync'] const execSyncShellMethods = ['execSync'] const childProcessChannel = dc.tracingChannel('datadog:child_process:execution') @@ -51,7 +56,7 @@ describe('child process', () => { }) }) - describe('async methods', (done) => { + describe('async methods', () => { describe('command not interpreted by a shell by default', () => { execAsyncMethods.forEach(methodName => { describe(`method ${methodName}`, () => { @@ -59,20 +64,59 @@ describe('child process', () => { const childEmitter = childProcess[methodName]('ls') childEmitter.once('close', () => { - expect(start).to.have.been.calledOnceWith({ command: 'ls', shell: false }) - expect(asyncFinish).to.have.been.calledOnceWith({ command: 'ls', shell: false, result: 0 }) + expect(start).to.have.been.calledOnceWith({ + command: 'ls', + file: 'ls', + shell: false, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(asyncFinish).to.have.been.calledOnceWith({ + command: 'ls', + file: 'ls', + shell: false, + result: 0 + }) expect(error).not.to.have.been.called done() }) }) + it('should publish arguments', (done) => { + const childEmitter = childProcess[methodName]('ls', ['-la']) + + childEmitter.once('close', () => { + expect(start).to.have.been.calledOnceWith({ + command: 'ls -la', + file: 'ls', + fileArgs: ['-la'], + shell: false, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(asyncFinish).to.have.been.calledOnceWith({ + command: 'ls -la', + file: 'ls', + shell: false, + fileArgs: ['-la'], + result: 0 + }) + + done() + }) + }) + it('should execute error callback', (done) => { const childEmitter = childProcess[methodName]('invalid_command_test') childEmitter.once('close', () => { - expect(start).to.have.been.calledOnceWith({ command: 'invalid_command_test', shell: false }) + expect(start).to.have.been.calledOnceWith({ + command: 'invalid_command_test', + file: 'invalid_command_test', + shell: false, + abortController: sinon.match.instanceOf(AbortController) + }) expect(asyncFinish).to.have.been.calledOnceWith({ command: 'invalid_command_test', + file: 'invalid_command_test', shell: false, result: -2 }) @@ -85,13 +129,20 @@ describe('child process', () => { const childEmitter = childProcess[methodName]('node -e "process.exit(1)"', { shell: true }) childEmitter.once('close', () => { - expect(start).to.have.been.calledOnceWith({ command: 'node -e "process.exit(1)"', shell: true }) + expect(start).to.have.been.calledOnceWith({ + command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', + abortController: sinon.match.instanceOf(AbortController), + shell: true + }) expect(asyncFinish).to.have.been.calledOnceWith({ command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', shell: true, result: 1 }) expect(error).to.have.been.calledOnce + done() }) }) @@ -101,13 +152,15 @@ describe('child process', () => { describe(`method ${methodName} with promisify`, () => { it('should execute success callbacks', async () => { await promisify(childProcess[methodName])('echo') + expect(start.firstCall.firstArg).to.include({ command: 'echo', + file: 'echo', shell: false }) - expect(asyncFinish).to.have.been.calledOnceWith({ command: 'echo', + file: 'echo', shell: false, result: { stdout: '\n', @@ -177,8 +230,13 @@ describe('child process', () => { const res = childProcess[methodName]('ls') res.once('close', () => { - expect(start).to.have.been.calledOnceWith({ command: 'ls', shell: true }) - expect(asyncFinish).to.have.been.calledOnceWith({ command: 'ls', shell: true, result: 0 }) + expect(start).to.have.been.calledOnceWith({ + command: 'ls', + file: 'ls', + shell: true, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(asyncFinish).to.have.been.calledOnceWith({ command: 'ls', file: 'ls', shell: true, result: 0 }) expect(error).not.to.have.been.called done() }) @@ -188,9 +246,15 @@ describe('child process', () => { const res = childProcess[methodName]('node -e "process.exit(1)"') res.once('close', () => { - expect(start).to.have.been.calledOnceWith({ command: 'node -e "process.exit(1)"', shell: true }) + expect(start).to.have.been.calledOnceWith({ + command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', + abortController: sinon.match.instanceOf(AbortController), + shell: true + }) expect(asyncFinish).to.have.been.calledOnceWith({ command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', shell: true, result: 1 }) @@ -203,10 +267,16 @@ describe('child process', () => { const res = childProcess[methodName]('invalid_command_test') res.once('close', () => { - expect(start).to.have.been.calledOnceWith({ command: 'invalid_command_test', shell: true }) + expect(start).to.have.been.calledOnceWith({ + command: 'invalid_command_test', + file: 'invalid_command_test', + abortController: sinon.match.instanceOf(AbortController), + shell: true + }) expect(error).to.have.been.calledOnce expect(asyncFinish).to.have.been.calledOnceWith({ command: 'invalid_command_test', + file: 'invalid_command_test', shell: true, result: 127 }) @@ -220,10 +290,13 @@ describe('child process', () => { await promisify(childProcess[methodName])('echo') expect(start).to.have.been.calledOnceWith({ command: 'echo', + file: 'echo', + abortController: sinon.match.instanceOf(AbortController), shell: true }) expect(asyncFinish).to.have.been.calledOnceWith({ command: 'echo', + file: 'echo', shell: true, result: 0 }) @@ -235,7 +308,12 @@ describe('child process', () => { await promisify(childProcess[methodName])('invalid_command_test') return Promise.reject(new Error('Command expected to fail')) } catch (e) { - expect(start).to.have.been.calledOnceWith({ command: 'invalid_command_test', shell: true }) + expect(start).to.have.been.calledOnceWith({ + command: 'invalid_command_test', + file: 'invalid_command_test', + abortController: sinon.match.instanceOf(AbortController), + shell: true + }) expect(asyncFinish).to.have.been.calledOnce expect(error).to.have.been.calledOnce } @@ -246,9 +324,15 @@ describe('child process', () => { await promisify(childProcess[methodName])('node -e "process.exit(1)"') return Promise.reject(new Error('Command expected to fail')) } catch (e) { - expect(start).to.have.been.calledOnceWith({ command: 'node -e "process.exit(1)"', shell: true }) + expect(start).to.have.been.calledOnceWith({ + command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', + abortController: sinon.match.instanceOf(AbortController), + shell: true + }) expect(asyncFinish).to.have.been.calledOnceWith({ command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', shell: true, result: 1 }) @@ -258,6 +342,58 @@ describe('child process', () => { }) }) }) + + describe('aborting in abortController', () => { + const abortError = new Error('AbortError') + function abort ({ abortController }) { + abortController.abort(abortError) + } + + beforeEach(() => { + childProcessChannel.subscribe({ start: abort }) + }) + + afterEach(() => { + childProcessChannel.unsubscribe({ start: abort }) + }) + + ;[...execAsyncMethods, ...execAsyncShellMethods].forEach((methodName) => { + describe(`method ${methodName}`, () => { + it('should execute callback with the error', (done) => { + childProcess[methodName]('aborted_command', (error) => { + expect(error).to.be.equal(abortError) + + done() + }) + }) + + it('should emit error and close', (done) => { + const cp = childProcess[methodName]('aborted_command') + const errorCallback = sinon.stub() + + cp.on('error', errorCallback) + cp.on('close', () => { + expect(errorCallback).to.have.been.calledWithExactly(abortError) + done() + }) + }) + + it('should emit error and close and execute the callback', (done) => { + const callback = sinon.stub() + const errorCallback = sinon.stub() + const cp = childProcess[methodName]('aborted_command', callback) + + cp.on('error', errorCallback) + cp.on('close', () => { + expect(callback).to.have.been.calledWithExactly(abortError) + expect(errorCallback).to.have.been.calledWithExactly(abortError) + + done() + }) + }) + }) + }) + }) }) describe('sync methods', () => { @@ -269,13 +405,15 @@ describe('child process', () => { expect(start).to.have.been.calledOnceWith({ command: 'ls', + file: 'ls', shell: false, - result + abortController: sinon.match.instanceOf(AbortController) }, 'tracing:datadog:child_process:execution:start') expect(finish).to.have.been.calledOnceWith({ command: 'ls', + file: 'ls', shell: false, result }, @@ -284,56 +422,105 @@ describe('child process', () => { expect(error).not.to.have.been.called }) - it('should execute error callback', () => { - let childError - try { - childProcess[methodName]('invalid_command_test') - } catch (error) { - childError = error - } finally { - expect(start).to.have.been.calledOnceWith({ - command: 'invalid_command_test', - shell: false, - error: childError - }) - expect(finish).to.have.been.calledOnce - expect(error).to.have.been.calledOnce - } - }) + it('should publish arguments', () => { + const result = childProcess[methodName]('ls', ['-la']) - it('should execute error callback with `exit 1` command', () => { - let childError - try { - childProcess[methodName]('node -e "process.exit(1)"') - } catch (error) { - childError = error - } finally { - expect(start).to.have.been.calledOnceWith({ - command: 'node -e "process.exit(1)"', - shell: false, - error: childError - }) - expect(finish).to.have.been.calledOnce - } + expect(start).to.have.been.calledOnceWith({ + command: 'ls -la', + file: 'ls', + shell: false, + fileArgs: ['-la'], + abortController: sinon.match.instanceOf(AbortController) + }) + expect(finish).to.have.been.calledOnceWith({ + command: 'ls -la', + file: 'ls', + shell: false, + fileArgs: ['-la'], + result + }) }) - if (methodName !== 'execFileSync' || NODE_MAJOR > 16) { - // when a process return an invalid code, in node <=16, in execFileSync with shell:true - // an exception is not thrown - it('should execute error callback with `exit 1` command with shell: true', () => { - let childError + + // errors are handled in a different way in spawnSync method + if (methodName !== 'spawnSync') { + it('should execute error callback', () => { + let childError, result try { - childProcess[methodName]('node -e "process.exit(1)"', { shell: true }) + result = childProcess[methodName]('invalid_command_test') } catch (error) { childError = error } finally { + childError = childError || result?.error + + const expectedContext = { + command: 'invalid_command_test', + file: 'invalid_command_test', + shell: false + } expect(start).to.have.been.calledOnceWith({ + ...expectedContext, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(finish).to.have.been.calledOnceWith({ + ...expectedContext, + error: childError + }) + expect(error).to.have.been.calledOnceWith({ + ...expectedContext, + error: childError + }) + } + }) + + it('should execute error callback with `exit 1` command', () => { + let childError + try { + childProcess[methodName]('node -e "process.exit(1)"') + } catch (error) { + childError = error + } finally { + const expectedContext = { command: 'node -e "process.exit(1)"', - shell: true, + file: 'node -e "process.exit(1)"', + shell: false + } + expect(start).to.have.been.calledOnceWith({ + ...expectedContext, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(finish).to.have.been.calledOnceWith({ + ...expectedContext, error: childError }) - expect(finish).to.have.been.calledOnce } }) + + if (methodName !== 'execFileSync' || NODE_MAJOR > 16) { + // when a process return an invalid code, in node <=16, in execFileSync with shell:true + // an exception is not thrown + it('should execute error callback with `exit 1` command with shell: true', () => { + let childError + try { + childProcess[methodName]('node -e "process.exit(1)"', { shell: true }) + } catch (error) { + childError = error + } finally { + const expectedContext = { + command: 'node -e "process.exit(1)"', + file: 'node -e "process.exit(1)"', + shell: true + } + expect(start).to.have.been.calledOnceWith({ + ...expectedContext, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(finish).to.have.been.calledOnceWith({ + ...expectedContext, + error: childError + }) + } + }) + } } }) }) @@ -345,14 +532,17 @@ describe('child process', () => { it('should execute success callbacks', () => { const result = childProcess[methodName]('ls') - expect(start).to.have.been.calledOnceWith({ + const expectedContext = { command: 'ls', - shell: true, - result + file: 'ls', + shell: true + } + expect(start).to.have.been.calledOnceWith({ + ...expectedContext, + abortController: sinon.match.instanceOf(AbortController) }) expect(finish).to.have.been.calledOnceWith({ - command: 'ls', - shell: true, + ...expectedContext, result }) expect(error).not.to.have.been.called @@ -365,13 +555,23 @@ describe('child process', () => { } catch (error) { childError = error } finally { - expect(start).to.have.been.calledOnceWith({ + const expectedContext = { command: 'invalid_command_test', - shell: true, + file: 'invalid_command_test', + shell: true + } + expect(start).to.have.been.calledOnceWith({ + ...expectedContext, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(finish).to.have.been.calledOnceWith({ + ...expectedContext, + error: childError + }) + expect(error).to.have.been.calledOnceWith({ + ...expectedContext, error: childError }) - expect(finish).to.have.been.calledOnce - expect(error).to.have.been.calledOnce } }) @@ -382,17 +582,71 @@ describe('child process', () => { } catch (error) { childError = error } finally { - expect(start).to.have.been.calledOnceWith({ + const expectedContext = { command: 'node -e "process.exit(1)"', - shell: true, + file: 'node -e "process.exit(1)"', + shell: true + } + expect(start).to.have.been.calledOnceWith({ + ...expectedContext, + abortController: sinon.match.instanceOf(AbortController) + }) + expect(finish).to.have.been.calledOnceWith({ + ...expectedContext, error: childError }) - expect(finish).to.have.been.calledOnce } }) }) }) }) + + describe('aborting in abortController', () => { + const abortError = new Error('AbortError') + function abort ({ abortController }) { + abortController.abort(abortError) + } + + beforeEach(() => { + childProcessChannel.subscribe({ start: abort }) + }) + + afterEach(() => { + childProcessChannel.unsubscribe({ start: abort }) + }) + + ;['execFileSync', 'execSync'].forEach((methodName) => { + describe(`method ${methodName}`, () => { + it('should throw the expected error', () => { + try { + childProcess[methodName]('aborted_command') + } catch (e) { + expect(e).to.be.equal(abortError) + + return + } + + throw new Error('Expected to fail') + }) + }) + }) + + describe('method spawnSync', () => { + it('should return error field', () => { + const result = childProcess.spawnSync('aborted_command') + + expect(result).to.be.deep.equal({ + error: abortError, + status: null, + signal: null, + output: null, + stdout: null, + stderr: null, + pid: 0 + }) + }) + }) + }) }) }) }) diff --git a/packages/dd-trace/src/appsec/addresses.js b/packages/dd-trace/src/appsec/addresses.js index 40c643012e..cb540bc4e6 100644 --- a/packages/dd-trace/src/appsec/addresses.js +++ b/packages/dd-trace/src/appsec/addresses.js @@ -28,6 +28,8 @@ module.exports = { DB_STATEMENT: 'server.db.statement', DB_SYSTEM: 'server.db.system', + SHELL_COMMAND: 'server.sys.shell.cmd', + LOGIN_SUCCESS: 'server.business_logic.users.login.success', LOGIN_FAILURE: 'server.business_logic.users.login.failure' } diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index 3081ed9974..897690652a 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -28,5 +28,6 @@ module.exports = { mysql2OuterQueryStart: dc.channel('datadog:mysql2:outerquery:start'), wafRunFinished: dc.channel('datadog:waf:run:finish'), fsOperationStart: dc.channel('apm:fs:operation:start'), - expressMiddlewareError: dc.channel('apm:express:middleware:error') + expressMiddlewareError: dc.channel('apm:express:middleware:error'), + childProcessExecutionTracingChannel: dc.tracingChannel('datadog:child_process:execution') } diff --git a/packages/dd-trace/src/appsec/rasp/command_injection.js b/packages/dd-trace/src/appsec/rasp/command_injection.js new file mode 100644 index 0000000000..76c7e47227 --- /dev/null +++ b/packages/dd-trace/src/appsec/rasp/command_injection.js @@ -0,0 +1,50 @@ +'use strict' + +const { childProcessExecutionTracingChannel } = require('../channels') +const { RULE_TYPES, handleResult } = require('./utils') +const { storage } = require('../../../../datadog-core') +const addresses = require('../addresses') +const waf = require('../waf') + +let config + +function enable (_config) { + config = _config + + childProcessExecutionTracingChannel.subscribe({ + start: analyzeCommandInjection + }) +} + +function disable () { + if (childProcessExecutionTracingChannel.start.hasSubscribers) { + childProcessExecutionTracingChannel.unsubscribe({ + start: analyzeCommandInjection + }) + } +} + +function analyzeCommandInjection ({ file, fileArgs, shell, abortController }) { + if (!file || !shell) return + + const store = storage.getStore() + const req = store?.req + if (!req) return + + // TODO remove join(' ') with new libddwaf + const commandParams = fileArgs ? [file, ...fileArgs].join(' ') : file + + const persistent = { + [addresses.SHELL_COMMAND]: commandParams + } + + const result = waf.run({ persistent }, req, RULE_TYPES.COMMAND_INJECTION) + + const res = store?.res + handleResult(result, req, res, abortController, config) +} + +module.exports = { + enable, + disable +} diff --git a/packages/dd-trace/src/appsec/rasp/index.js b/packages/dd-trace/src/appsec/rasp/index.js index d5a1312872..4a65518495 100644 --- a/packages/dd-trace/src/appsec/rasp/index.js +++ b/packages/dd-trace/src/appsec/rasp/index.js @@ -6,6 +6,7 @@ const { block, isBlocked } = require('../blocking') const ssrf = require('./ssrf') const sqli = require('./sql_injection') const lfi = require('./lfi') +const cmdi = require('./command_injection') const { DatadogRaspAbortError } = require('./utils') @@ -95,6 +96,7 @@ function enable (config) { ssrf.enable(config) sqli.enable(config) lfi.enable(config) + cmdi.enable(config) process.on('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor) expressMiddlewareError.subscribe(blockOnDatadogRaspAbortError) @@ -104,6 +106,7 @@ function disable () { ssrf.disable() sqli.disable() lfi.disable() + cmdi.disable() process.off('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor) if (expressMiddlewareError.hasSubscribers) expressMiddlewareError.unsubscribe(blockOnDatadogRaspAbortError) diff --git a/packages/dd-trace/src/appsec/rasp/utils.js b/packages/dd-trace/src/appsec/rasp/utils.js index c4ee4f55c3..bdf3596209 100644 --- a/packages/dd-trace/src/appsec/rasp/utils.js +++ b/packages/dd-trace/src/appsec/rasp/utils.js @@ -12,9 +12,10 @@ if (abortOnUncaughtException) { } const RULE_TYPES = { - SSRF: 'ssrf', + COMMAND_INJECTION: 'command_injection', + LFI: 'lfi', SQL_INJECTION: 'sql_injection', - LFI: 'lfi' + SSRF: 'ssrf' } class DatadogRaspAbortError extends Error { diff --git a/packages/dd-trace/test/appsec/rasp/command_injection.express.plugin.spec.js b/packages/dd-trace/test/appsec/rasp/command_injection.express.plugin.spec.js new file mode 100644 index 0000000000..c9c99d3fee --- /dev/null +++ b/packages/dd-trace/test/appsec/rasp/command_injection.express.plugin.spec.js @@ -0,0 +1,432 @@ +'use strict' + +const agent = require('../../plugins/agent') +const appsec = require('../../../src/appsec') +const Config = require('../../../src/config') +const path = require('path') +const Axios = require('axios') +const { getWebSpan, checkRaspExecutedAndHasThreat, checkRaspExecutedAndNotThreat } = require('./utils') +const { assert } = require('chai') + +describe('RASP - command_injection', () => { + withVersions('express', 'express', expressVersion => { + let app, server, axios + + async function testBlockingRequest () { + try { + await axios.get('/?dir=$(cat /etc/passwd 1>%262 ; echo .)') + } catch (e) { + if (!e.response) { + throw e + } + + return checkRaspExecutedAndHasThreat(agent, 'rasp-command_injection-rule-id-3') + } + + assert.fail('Request should be blocked') + } + + function checkRaspNotExecutedAndNotThreat (agent, checkRuleEval = true) { + return agent.use((traces) => { + const span = getWebSpan(traces) + assert.notProperty(span.meta, '_dd.appsec.json') + assert.notProperty(span.meta_struct || {}, '_dd.stack') + if (checkRuleEval) { + assert.notProperty(span.metrics, '_dd.appsec.rasp.rule.eval') + } + }) + } + + function testBlockingAndSafeRequests () { + it('should block the threat', async () => { + await testBlockingRequest() + }) + + it('should not block safe request', async () => { + await axios.get('/?dir=.') + + return checkRaspExecutedAndNotThreat(agent) + }) + } + + function testSafeInNonShell () { + it('should not block the threat', async () => { + await axios.get('/?dir=$(cat /etc/passwd 1>%262 ; echo .)') + + return checkRaspNotExecutedAndNotThreat(agent) + }) + + it('should not block safe request', async () => { + await axios.get('/?dir=.') + + return checkRaspNotExecutedAndNotThreat(agent) + }) + } + + before(() => { + return agent.load(['express', 'http', 'child_process'], { client: false }) + }) + + before((done) => { + const express = require(`../../../../../versions/express@${expressVersion}`).get() + const expressApp = express() + + expressApp.get('/', (req, res) => { + app(req, res) + }) + + appsec.enable(new Config({ + appsec: { + enabled: true, + rules: path.join(__dirname, 'resources', 'rasp_rules.json'), + rasp: { enabled: true } + } + })) + + server = expressApp.listen(0, () => { + const port = server.address().port + axios = Axios.create({ + baseURL: `http://localhost:${port}` + }) + done() + }) + }) + + after(() => { + appsec.disable() + server.close() + return agent.close({ ritmReset: false }) + }) + + describe('exec', () => { + describe('with callback', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + childProcess.exec(`ls ${req.query.dir}`, function (e) { + if (e?.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + + res.end('end') + }) + } + }) + + testBlockingAndSafeRequests() + }) + + describe('with promise', () => { + beforeEach(() => { + app = async (req, res) => { + const util = require('util') + const exec = util.promisify(require('child_process').exec) + + try { + await exec(`ls ${req.query.dir}`) + } catch (e) { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + } + + res.end('end') + } + }) + + testBlockingAndSafeRequests() + }) + + describe('with event emitter', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.exec(`ls ${req.query.dir}`) + child.on('error', (e) => { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + }) + + child.on('close', () => { + res.end() + }) + } + }) + + testBlockingAndSafeRequests() + }) + + describe('execSync', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + try { + childProcess.execSync(`ls ${req.query.dir}`) + } catch (e) { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + } + + res.end('end') + } + }) + + testBlockingAndSafeRequests() + }) + }) + + describe('execFile', () => { + // requires new libddwaf with support for array + describe('with shell: true', () => { + describe('with callback', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + childProcess.execFile('ls', [req.query.dir], { shell: true }, function (e) { + if (e?.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + + res.end('end') + }) + } + }) + + testBlockingAndSafeRequests() + }) + + describe('with promise', () => { + beforeEach(() => { + app = async (req, res) => { + const util = require('util') + const execFile = util.promisify(require('child_process').execFile) + + try { + await execFile('ls', [req.query.dir], { shell: true }) + } catch (e) { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + } + + res.end('end') + } + }) + + testBlockingAndSafeRequests() + }) + + describe('with event emitter', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.execFile('ls', [req.query.dir], { shell: true }) + child.on('error', (e) => { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + }) + + child.on('close', () => { + res.end() + }) + } + }) + + testBlockingAndSafeRequests() + }) + + describe('execFileSync', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + try { + childProcess.execFileSync('ls', [req.query.dir], { shell: true }) + } catch (e) { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + } + + res.end() + } + }) + + testBlockingAndSafeRequests() + }) + }) + + describe('without shell', () => { + describe('with callback', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + childProcess.execFile('ls', [req.query.dir], function (e) { + if (e?.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + + res.end('end') + }) + } + }) + + testSafeInNonShell() + }) + + describe('with promise', () => { + beforeEach(() => { + app = async (req, res) => { + const util = require('util') + const execFile = util.promisify(require('child_process').execFile) + + try { + await execFile('ls', [req.query.dir]) + } catch (e) { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + } + + res.end('end') + } + }) + + testSafeInNonShell() + }) + + describe('with event emitter', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.execFile('ls', [req.query.dir]) + child.on('error', (e) => { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + }) + + child.on('close', () => { + res.end() + }) + } + }) + + testSafeInNonShell() + }) + + describe('execFileSync', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + try { + childProcess.execFileSync('ls', [req.query.dir]) + } catch (e) { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + } + + res.end() + } + }) + + testSafeInNonShell() + }) + }) + }) + + describe('spawn', () => { + // requires new libddwaf with support for array + describe('with shell: true', () => { + describe('with event emitter', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.spawn('ls', [req.query.dir], { shell: true }) + child.on('error', (e) => { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + }) + + child.on('close', () => { + res.end() + }) + } + }) + + testBlockingAndSafeRequests() + }) + + describe('spawnSync', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.spawnSync('ls', [req.query.dir], { shell: true }) + if (child.error?.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + + res.end() + } + }) + + testBlockingAndSafeRequests() + }) + }) + + describe('without shell', () => { + describe('with event emitter', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.spawn('ls', [req.query.dir]) + child.on('error', (e) => { + if (e.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + }) + + child.on('close', () => { + res.end() + }) + } + }) + + testSafeInNonShell() + }) + + describe('spawnSync', () => { + beforeEach(() => { + app = (req, res) => { + const childProcess = require('child_process') + + const child = childProcess.spawnSync('ls', [req.query.dir]) + if (child.error?.name === 'DatadogRaspAbortError') { + res.writeHead(500) + } + + res.end() + } + }) + + testSafeInNonShell() + }) + }) + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/rasp/command_injection.spec.js b/packages/dd-trace/test/appsec/rasp/command_injection.spec.js new file mode 100644 index 0000000000..b726fef5fa --- /dev/null +++ b/packages/dd-trace/test/appsec/rasp/command_injection.spec.js @@ -0,0 +1,137 @@ +'use strict' + +const proxyquire = require('proxyquire') +const { childProcessExecutionStart } = require('../../../src/appsec/channels') +const addresses = require('../../../src/appsec/addresses') + +describe('RASP - command_injection.js', () => { + let waf, datadogCore, commandInjection, utils, config + + beforeEach(() => { + datadogCore = { + storage: { + getStore: sinon.stub() + } + } + + waf = { + run: sinon.stub() + } + + utils = { + handleResult: sinon.stub() + } + + commandInjection = proxyquire('../../../src/appsec/rasp/command_injection', { + '../../../../datadog-core': datadogCore, + '../waf': waf, + './utils': utils + }) + + config = { + appsec: { + stackTrace: { + enabled: true, + maxStackTraces: 2, + maxDepth: 42 + } + } + } + + commandInjection.enable(config) + }) + + afterEach(() => { + sinon.restore() + commandInjection.disable() + }) + + describe('analyzeCommandInjection', () => { + it('should analyze command_injection without arguments', () => { + const ctx = { + file: 'cmd' + } + const req = {} + datadogCore.storage.getStore.returns({ req }) + + childProcessExecutionStart.publish(ctx) + + const persistent = { [addresses.SHELL_COMMAND]: 'cmd' } + sinon.assert.calledOnceWithExactly(waf.run, { persistent }, req, 'command_injection') + }) + + it('should analyze command_injection with arguments', () => { + const ctx = { + file: 'cmd', + fileArgs: ['arg0', 'arg1'] + } + const req = {} + datadogCore.storage.getStore.returns({ req }) + + childProcessExecutionStart.publish(ctx) + + const persistent = { [addresses.SHELL_COMMAND]: ['cmd', 'arg0', 'arg1'] } + sinon.assert.calledOnceWithExactly(waf.run, { persistent }, req, 'command_injection') + }) + + it('should not analyze command_injection if rasp is disabled', () => { + commandInjection.disable() + const ctx = { + file: 'cmd' + } + const req = {} + datadogCore.storage.getStore.returns({ req }) + + childProcessExecutionStart.publish(ctx) + + sinon.assert.notCalled(waf.run) + }) + + it('should not analyze command_injection if no store', () => { + const ctx = { + file: 'cmd' + } + datadogCore.storage.getStore.returns(undefined) + + childProcessExecutionStart.publish(ctx) + + sinon.assert.notCalled(waf.run) + }) + + it('should not analyze command_injection if no req', () => { + const ctx = { + file: 'cmd' + } + datadogCore.storage.getStore.returns({}) + + childProcessExecutionStart.publish(ctx) + + sinon.assert.notCalled(waf.run) + }) + + it('should not analyze command_injection if no file', () => { + const ctx = { + fileArgs: ['arg0'] + } + datadogCore.storage.getStore.returns({}) + + childProcessExecutionStart.publish(ctx) + + sinon.assert.notCalled(waf.run) + }) + + it('should call handleResult', () => { + const abortController = { abort: 'abort' } + const ctx = { file: 'cmd', abortController } + 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) + + sinon.assert.calledOnceWithExactly(utils.handleResult, wafResult, req, res, abortController, config) + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json b/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json index 778e4821e7..daca47d8d2 100644 --- a/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json +++ b/packages/dd-trace/test/appsec/rasp/resources/rasp_rules.json @@ -107,6 +107,55 @@ "block", "stack_trace" ] + }, + { + "id": "rasp-command_injection-rule-id-3", + "name": "Command injection exploit", + "tags": { + "type": "command_injection", + "category": "vulnerability_trigger", + "cwe": "77", + "capec": "1000/152/248/88", + "confidence": "0", + "module": "rasp" + }, + "conditions": [ + { + "parameters": { + "resource": [ + { + "address": "server.sys.shell.cmd" + } + ], + "params": [ + { + "address": "server.request.query" + }, + { + "address": "server.request.body" + }, + { + "address": "server.request.path_params" + }, + { + "address": "grpc.server.request.message" + }, + { + "address": "graphql.server.all_resolvers" + }, + { + "address": "graphql.server.resolver" + } + ] + }, + "operator": "shi_detector" + } + ], + "transformers": [], + "on_match": [ + "block", + "stack_trace" + ] } ] }