Skip to content

Commit

Permalink
Shell injection exploit prevention
Browse files Browse the repository at this point in the history
  • Loading branch information
uurien committed Oct 22, 2024
1 parent e7edfcf commit 97fd1c8
Show file tree
Hide file tree
Showing 10 changed files with 1,137 additions and 94 deletions.
168 changes: 141 additions & 27 deletions packages/datadog-instrumentations/src/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']

Expand All @@ -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
Expand All @@ -34,25 +35,29 @@ 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'

return childProcessInfo
}

function wrapChildProcessSyncMethod (shell = false) {
function wrapChildProcessSyncMethod (methodName, shell = false) {
return function wrapMethod (childProcessMethod) {
return function () {
if (!childProcessChannel.start.hasSubscribers || arguments.length === 0) {
Expand All @@ -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)
}
})
}
}
Expand All @@ -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) {
Expand All @@ -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

Expand All @@ -129,8 +244,7 @@ function wrapChildProcessAsyncMethod (shell = false) {
childProcessChannel.error.publish()
}
childProcessChannel.asyncEnd.publish({
command: childProcessInfo.command,
shell: childProcessInfo.shell,
...context,
result: code
})
})
Expand Down
Loading

0 comments on commit 97fd1c8

Please sign in to comment.