From 0a307598c42eeb54d09fca0d1ed922dcede03e15 Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 8 Jan 2025 14:42:19 +0100 Subject: [PATCH 1/6] Instrument vm for code injection vulnerability --- .../src/helpers/hooks.js | 2 + packages/datadog-instrumentations/src/vm.js | 41 ++ .../iast/analyzers/code-injection-analyzer.js | 1 + ...-injection-analyzer.express.plugin.spec.js | 400 +++++++++++++++--- 4 files changed, 394 insertions(+), 50 deletions(-) create mode 100644 packages/datadog-instrumentations/src/vm.js diff --git a/packages/datadog-instrumentations/src/helpers/hooks.js b/packages/datadog-instrumentations/src/helpers/hooks.js index 4ea35f50218..13dda1145bf 100644 --- a/packages/datadog-instrumentations/src/helpers/hooks.js +++ b/packages/datadog-instrumentations/src/helpers/hooks.js @@ -96,6 +96,7 @@ module.exports = { 'node:https': () => require('../http'), 'node:net': () => require('../net'), 'node:url': () => require('../url'), + 'node:vm': () => require('../vm'), nyc: () => require('../nyc'), oracledb: () => require('../oracledb'), openai: () => require('../openai'), @@ -122,6 +123,7 @@ module.exports = { undici: () => require('../undici'), url: () => require('../url'), vitest: { esmFirst: true, fn: () => require('../vitest') }, + vm: () => require('../vm'), when: () => require('../when'), winston: () => require('../winston'), workerpool: () => require('../mocha') diff --git a/packages/datadog-instrumentations/src/vm.js b/packages/datadog-instrumentations/src/vm.js new file mode 100644 index 00000000000..d91f89cd48a --- /dev/null +++ b/packages/datadog-instrumentations/src/vm.js @@ -0,0 +1,41 @@ +'use strict' + +const { channel, addHook } = require('./helpers/instrument') +const shimmer = require('../../datadog-shimmer') +const names = ['vm', 'node:vm'] + +const runScriptStartChannel = channel('datadog:vm:run-script:start') + +addHook({ name: names }, function (vm) { + vm.Script = class extends vm.Script { + constructor (code) { + super(...arguments) + this.code = code + } + } + + shimmer.wrap(vm.Script.prototype, 'runInContext', wrapVMMethod(1)) + shimmer.wrap(vm.Script.prototype, 'runInNewContext', wrapVMMethod()) + shimmer.wrap(vm.Script.prototype, 'runInThisContext', wrapVMMethod()) + + shimmer.wrap(vm, 'runInContext', wrapVMMethod()) + shimmer.wrap(vm, 'runInNewContext', wrapVMMethod()) + shimmer.wrap(vm, 'runInThisContext', wrapVMMethod()) + shimmer.wrap(vm, 'compileFunction', wrapVMMethod()) + + return vm +}) + +function wrapVMMethod (codeIndex = 0) { + return function wrap (original) { + return function wrapped () { + const code = arguments[codeIndex] ? arguments[codeIndex] : this.code + + if (runScriptStartChannel.hasSubscribers && code) { + runScriptStartChannel.publish({ code }) + } + + return original.apply(this, arguments) + } + } +} diff --git a/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js index 3741c12ef8f..c379c2ae8fe 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js @@ -10,6 +10,7 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer { onConfigure () { this.addSub('datadog:eval:call', ({ script }) => this.analyze(script)) + this.addSub('datadog:vm:run-script:start', ({ code }) => this.analyze(code)) } _areRangesVulnerable () { diff --git a/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js index 64e15b9161b..44cfd113c48 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js @@ -13,64 +13,364 @@ const iastContextFunctions = require('../../../../src/appsec/iast/iast-context') describe('Code injection vulnerability', () => { withVersions('express', 'express', '>4.18.0', version => { - let i = 0 - let evalFunctionsPath - - beforeEach(() => { - evalFunctionsPath = path.join(os.tmpdir(), `eval-methods-${i++}.js`) - fs.copyFileSync( - path.join(__dirname, 'resources', 'eval-methods.js'), - evalFunctionsPath - ) - }) + describe('Eval', () => { + let i = 0 + let evalFunctionsPath + + beforeEach(() => { + evalFunctionsPath = path.join(os.tmpdir(), `eval-methods-${i++}.js`) + fs.copyFileSync( + path.join(__dirname, 'resources', 'eval-methods.js'), + evalFunctionsPath + ) + }) + + afterEach(() => { + fs.unlinkSync(evalFunctionsPath) + clearCache() + }) + + prepareTestServerForIastInExpress('in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + res.send(require(evalFunctionsPath).runEval(req.query.script, 'test-result')) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal('test-result') + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + res.send(require(evalFunctionsPath).runEval(str, 'test-result')) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) - afterEach(() => { - fs.unlinkSync(evalFunctionsPath) - clearCache() + testThatRequestHasNoVulnerability({ + fn: (req, res) => { + res.send('' + require(evalFunctionsPath).runFakeEval(req.query.script)) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`).catch(done) + } + }) + + testThatRequestHasNoVulnerability((req, res) => { + res.send('' + require(evalFunctionsPath).runEval('1 + 2')) + }, 'CODE_INJECTION') + }) }) - prepareTestServerForIastInExpress('in express', version, - (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { - testThatRequestHasVulnerability({ - fn: (req, res) => { - res.send(require(evalFunctionsPath).runEval(req.query.script, 'test-result')) - }, - vulnerability: 'CODE_INJECTION', - makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=1%2B2`) - .then(res => { - expect(res.data).to.equal('test-result') - }) - .catch(done) - } + describe('Node:vm', () => { + let context, vm + + beforeEach(() => { + vm = require('vm') + context = {} + vm.createContext(context) + }) + + afterEach(() => { + vm = null + context = null + }) + + prepareTestServerForIastInExpress('runInContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const result = vm.runInContext(req.query.script, context) + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const result = vm.runInContext(str, context) + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const result = vm.runInContext('1 + 2', context) + + res.send(`${result}`) + }, 'CODE_INJECTION') }) - testThatRequestHasVulnerability({ - fn: (req, res) => { - const source = '1 + 2' - const store = storage.getStore() - const iastContext = iastContextFunctions.getIastContext(store) - const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) - - res.send(require(evalFunctionsPath).runEval(str, 'test-result')) - }, - vulnerability: 'CODE_INJECTION', - testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + prepareTestServerForIastInExpress('Script runInContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const script = new vm.Script(req.query.script) + const result = script.runInContext(context) + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const script = new vm.Script(str) + const result = script.runInContext(context) + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const script = new vm.Script('1 + 2') + const result = script.runInContext(context) + + res.send(`${result}`) + }, 'CODE_INJECTION') }) - testThatRequestHasNoVulnerability({ - fn: (req, res) => { - res.send('' + require(evalFunctionsPath).runFakeEval(req.query.script)) - }, - vulnerability: 'CODE_INJECTION', - makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=1%2B2`).catch(done) - } + prepareTestServerForIastInExpress('runInNewContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const result = vm.runInNewContext(req.query.script) + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const result = vm.runInNewContext(str) + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const result = vm.runInNewContext('1 + 2') + + res.send(`${result}`) + }, 'CODE_INJECTION') }) - testThatRequestHasNoVulnerability((req, res) => { - res.send('' + require(evalFunctionsPath).runEval('1 + 2')) - }, 'CODE_INJECTION') - }) + prepareTestServerForIastInExpress('Script runInNewContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const script = new vm.Script(req.query.script) + const result = script.runInNewContext() + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const script = new vm.Script(str) + const result = script.runInNewContext() + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const script = new vm.Script('1 + 2') + const result = script.runInNewContext() + + res.send(`${result}`) + }, 'CODE_INJECTION') + }) + + prepareTestServerForIastInExpress('runInThisContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const result = vm.runInThisContext(req.query.script) + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const result = vm.runInThisContext(str) + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const result = vm.runInThisContext('1 + 2') + + res.send(`${result}`) + }, 'CODE_INJECTION') + }) + + prepareTestServerForIastInExpress('Script runInThisContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const script = new vm.Script(req.query.script) + const result = script.runInThisContext() + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const script = new vm.Script(str) + const result = script.runInThisContext() + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const script = new vm.Script('1 + 2') + const result = script.runInThisContext() + + res.send(`${result}`) + }, 'CODE_INJECTION') + }) + + prepareTestServerForIastInExpress('compileFunction in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const fn = vm.compileFunction(req.query.script) + const result = fn() + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=return%201%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const result = vm.runInThisContext(str) + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const result = vm.runInThisContext('1 + 2') + + res.send(`${result}`) + }, 'CODE_INJECTION') + }) + }) }) }) From 2bfb6034c53bf7877d724e79cdda258673fda537 Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 9 Jan 2025 11:33:56 +0100 Subject: [PATCH 2/6] simplify vm constructor instrumentation --- packages/datadog-instrumentations/src/vm.js | 33 +++++++++------------ 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/packages/datadog-instrumentations/src/vm.js b/packages/datadog-instrumentations/src/vm.js index d91f89cd48a..0c6fdfa5c88 100644 --- a/packages/datadog-instrumentations/src/vm.js +++ b/packages/datadog-instrumentations/src/vm.js @@ -10,32 +10,27 @@ addHook({ name: names }, function (vm) { vm.Script = class extends vm.Script { constructor (code) { super(...arguments) - this.code = code + + if (runScriptStartChannel.hasSubscribers && code) { + runScriptStartChannel.publish({ code }) + } } } - shimmer.wrap(vm.Script.prototype, 'runInContext', wrapVMMethod(1)) - shimmer.wrap(vm.Script.prototype, 'runInNewContext', wrapVMMethod()) - shimmer.wrap(vm.Script.prototype, 'runInThisContext', wrapVMMethod()) - - shimmer.wrap(vm, 'runInContext', wrapVMMethod()) - shimmer.wrap(vm, 'runInNewContext', wrapVMMethod()) - shimmer.wrap(vm, 'runInThisContext', wrapVMMethod()) - shimmer.wrap(vm, 'compileFunction', wrapVMMethod()) + shimmer.wrap(vm, 'runInContext', wrapVMMethod) + shimmer.wrap(vm, 'runInNewContext', wrapVMMethod) + shimmer.wrap(vm, 'runInThisContext', wrapVMMethod) + shimmer.wrap(vm, 'compileFunction', wrapVMMethod) return vm }) -function wrapVMMethod (codeIndex = 0) { - return function wrap (original) { - return function wrapped () { - const code = arguments[codeIndex] ? arguments[codeIndex] : this.code - - if (runScriptStartChannel.hasSubscribers && code) { - runScriptStartChannel.publish({ code }) - } - - return original.apply(this, arguments) +function wrapVMMethod (original) { + return function wrappedVMMethod (code) { + if (runScriptStartChannel.hasSubscribers && code) { + runScriptStartChannel.publish({ code }) } + + return original.apply(this, arguments) } } From a6f076cfa09920ec8b92119a4cb31183438d85dc Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 15 Jan 2025 15:33:13 +0100 Subject: [PATCH 3/6] support SourceTextModule class --- package.json | 2 +- packages/datadog-instrumentations/src/vm.js | 11 + .../iast/analyzers/code-injection-analyzer.js | 1 + ...-injection-analyzer.express.plugin.spec.js | 293 ++++++++++-------- 4 files changed, 185 insertions(+), 122 deletions(-) diff --git a/package.json b/package.json index fedd38e7312..6bb87d3ad5c 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "test": "SERVICES=* yarn services && mocha --expose-gc 'packages/dd-trace/test/setup/node.js' 'packages/*/test/**/*.spec.js'", "test:appsec": "mocha -r \"packages/dd-trace/test/setup/mocha.js\" --exclude \"packages/dd-trace/test/appsec/**/*.plugin.spec.js\" \"packages/dd-trace/test/appsec/**/*.spec.js\"", "test:appsec:ci": "nyc --no-clean --include \"packages/dd-trace/src/appsec/**/*.js\" --exclude \"packages/dd-trace/test/appsec/**/*.plugin.spec.js\" -- npm run test:appsec", - "test:appsec:plugins": "mocha -r \"packages/dd-trace/test/setup/mocha.js\" \"packages/dd-trace/test/appsec/**/*.@($(echo $PLUGINS)).plugin.spec.js\"", + "test:appsec:plugins": "mocha -r \"packages/dd-trace/test/setup/mocha.js\" --experimental-vm-modules \"packages/dd-trace/test/appsec/**/*.@($(echo $PLUGINS)).plugin.spec.js\"", "test:appsec:plugins:ci": "yarn services && nyc --no-clean --include \"packages/dd-trace/src/appsec/**/*.js\" -- npm run test:appsec:plugins", "test:debugger": "mocha -r 'packages/dd-trace/test/setup/mocha.js' 'packages/dd-trace/test/debugger/**/*.spec.js'", "test:debugger:ci": "nyc --no-clean --include 'packages/dd-trace/src/debugger/**/*.js' -- npm run test:debugger", diff --git a/packages/datadog-instrumentations/src/vm.js b/packages/datadog-instrumentations/src/vm.js index 0c6fdfa5c88..7bca1787481 100644 --- a/packages/datadog-instrumentations/src/vm.js +++ b/packages/datadog-instrumentations/src/vm.js @@ -5,6 +5,7 @@ const shimmer = require('../../datadog-shimmer') const names = ['vm', 'node:vm'] const runScriptStartChannel = channel('datadog:vm:run-script:start') +const sourceTextModuleStartChannel = channel('datadog:vm:source-text-module:start') addHook({ name: names }, function (vm) { vm.Script = class extends vm.Script { @@ -17,6 +18,16 @@ addHook({ name: names }, function (vm) { } } + vm.SourceTextModule = class extends vm.SourceTextModule { + constructor (sourceText) { + super(...arguments) + + if (sourceTextModuleStartChannel.hasSubscribers && sourceText) { + sourceTextModuleStartChannel.publish({ sourceText }) + } + } + } + shimmer.wrap(vm, 'runInContext', wrapVMMethod) shimmer.wrap(vm, 'runInNewContext', wrapVMMethod) shimmer.wrap(vm, 'runInThisContext', wrapVMMethod) diff --git a/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js index c379c2ae8fe..fe36d83482b 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js @@ -11,6 +11,7 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer { onConfigure () { this.addSub('datadog:eval:call', ({ script }) => this.analyze(script)) this.addSub('datadog:vm:run-script:start', ({ code }) => this.analyze(code)) + this.addSub('datadog:vm:source-text-module:start', ({ sourceText }) => this.analyze(sourceText)) } _areRangesVulnerable () { diff --git a/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js index 44cfd113c48..840159fd63f 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js @@ -12,7 +12,7 @@ const { storage } = require('../../../../../datadog-core') const iastContextFunctions = require('../../../../src/appsec/iast/iast-context') describe('Code injection vulnerability', () => { - withVersions('express', 'express', '>4.18.0', version => { + withVersions('express', 'express', version => { describe('Eval', () => { let i = 0 let evalFunctionsPath @@ -128,48 +128,6 @@ describe('Code injection vulnerability', () => { }, 'CODE_INJECTION') }) - prepareTestServerForIastInExpress('Script runInContext in express', version, - (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { - testThatRequestHasVulnerability({ - fn: (req, res) => { - const script = new vm.Script(req.query.script) - const result = script.runInContext(context) - - res.send(`${result}`) - }, - vulnerability: 'CODE_INJECTION', - makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=1%2B2`) - .then(res => { - expect(res.data).to.equal(3) - }) - .catch(done) - } - }) - - testThatRequestHasVulnerability({ - fn: (req, res) => { - const source = '1 + 2' - const store = storage.getStore() - const iastContext = iastContextFunctions.getIastContext(store) - const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) - - const script = new vm.Script(str) - const result = script.runInContext(context) - res.send(`${result}`) - }, - vulnerability: 'CODE_INJECTION', - testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' - }) - - testThatRequestHasNoVulnerability((req, res) => { - const script = new vm.Script('1 + 2') - const result = script.runInContext(context) - - res.send(`${result}`) - }, 'CODE_INJECTION') - }) - prepareTestServerForIastInExpress('runInNewContext in express', version, (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { testThatRequestHasVulnerability({ @@ -209,12 +167,11 @@ describe('Code injection vulnerability', () => { }, 'CODE_INJECTION') }) - prepareTestServerForIastInExpress('Script runInNewContext in express', version, + prepareTestServerForIastInExpress('runInThisContext in express', version, (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { testThatRequestHasVulnerability({ fn: (req, res) => { - const script = new vm.Script(req.query.script) - const result = script.runInNewContext() + const result = vm.runInThisContext(req.query.script) res.send(`${result}`) }, @@ -235,8 +192,7 @@ describe('Code injection vulnerability', () => { const iastContext = iastContextFunctions.getIastContext(store) const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) - const script = new vm.Script(str) - const result = script.runInNewContext() + const result = vm.runInThisContext(str) res.send(`${result}`) }, vulnerability: 'CODE_INJECTION', @@ -244,24 +200,24 @@ describe('Code injection vulnerability', () => { }) testThatRequestHasNoVulnerability((req, res) => { - const script = new vm.Script('1 + 2') - const result = script.runInNewContext() + const result = vm.runInThisContext('1 + 2') res.send(`${result}`) }, 'CODE_INJECTION') }) - prepareTestServerForIastInExpress('runInThisContext in express', version, + prepareTestServerForIastInExpress('compileFunction in express', version, (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { testThatRequestHasVulnerability({ fn: (req, res) => { - const result = vm.runInThisContext(req.query.script) + const fn = vm.compileFunction(req.query.script) + const result = fn() res.send(`${result}`) }, vulnerability: 'CODE_INJECTION', makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=1%2B2`) + axios.get(`http://localhost:${config.port}/?script=return%201%2B2`) .then(res => { expect(res.data).to.equal(3) }) @@ -290,87 +246,182 @@ describe('Code injection vulnerability', () => { }, 'CODE_INJECTION') }) - prepareTestServerForIastInExpress('Script runInThisContext in express', version, - (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { - testThatRequestHasVulnerability({ - fn: (req, res) => { - const script = new vm.Script(req.query.script) - const result = script.runInThisContext() + describe('Script class', () => { + prepareTestServerForIastInExpress('runInContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const script = new vm.Script(req.query.script) + const result = script.runInContext(context) + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const script = new vm.Script(str) + const result = script.runInContext(context) + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const script = new vm.Script('1 + 2') + const result = script.runInContext(context) res.send(`${result}`) - }, - vulnerability: 'CODE_INJECTION', - makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=1%2B2`) - .then(res => { - expect(res.data).to.equal(3) - }) - .catch(done) - } + }, 'CODE_INJECTION') }) - testThatRequestHasVulnerability({ - fn: (req, res) => { - const source = '1 + 2' - const store = storage.getStore() - const iastContext = iastContextFunctions.getIastContext(store) - const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + prepareTestServerForIastInExpress('runInNewContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const script = new vm.Script(req.query.script) + const result = script.runInNewContext() + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const script = new vm.Script(str) + const result = script.runInNewContext() + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const script = new vm.Script('1 + 2') + const result = script.runInNewContext() - const script = new vm.Script(str) - const result = script.runInThisContext() res.send(`${result}`) - }, - vulnerability: 'CODE_INJECTION', - testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }, 'CODE_INJECTION') }) - testThatRequestHasNoVulnerability((req, res) => { - const script = new vm.Script('1 + 2') - const result = script.runInThisContext() - - res.send(`${result}`) - }, 'CODE_INJECTION') - }) - - prepareTestServerForIastInExpress('compileFunction in express', version, - (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { - testThatRequestHasVulnerability({ - fn: (req, res) => { - const fn = vm.compileFunction(req.query.script) - const result = fn() + prepareTestServerForIastInExpress('runInThisContext in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: (req, res) => { + const script = new vm.Script(req.query.script) + const result = script.runInThisContext() + + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=1%2B2`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: (req, res) => { + const source = '1 + 2' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const script = new vm.Script(str) + const result = script.runInThisContext() + res.send(`${result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability((req, res) => { + const script = new vm.Script('1 + 2') + const result = script.runInThisContext() res.send(`${result}`) - }, - vulnerability: 'CODE_INJECTION', - makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=return%201%2B2`) - .then(res => { - expect(res.data).to.equal(3) - }) - .catch(done) - } + }, 'CODE_INJECTION') }) + }) - testThatRequestHasVulnerability({ - fn: (req, res) => { - const source = '1 + 2' - const store = storage.getStore() - const iastContext = iastContextFunctions.getIastContext(store) - const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) - - const result = vm.runInThisContext(str) - res.send(`${result}`) - }, - vulnerability: 'CODE_INJECTION', - testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + describe('SourceTextModule class', () => { + prepareTestServerForIastInExpress('SourceTextModule in express', version, + (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { + testThatRequestHasVulnerability({ + fn: async (req, res) => { + const module = new vm.SourceTextModule(req.query.script) + await module.link(() => {}) + await module.evaluate() + + res.send(`${module.namespace.result}`) + }, + vulnerability: 'CODE_INJECTION', + makeRequest: (done, config) => { + axios.get(`http://localhost:${config.port}/?script=export%20const%20result%20%3D%203%3B`) + .then(res => { + expect(res.data).to.equal(3) + }) + .catch(done) + } + }) + + testThatRequestHasVulnerability({ + fn: async (req, res) => { + const source = 'export const result = 1 + 2;' + const store = storage.getStore() + const iastContext = iastContextFunctions.getIastContext(store) + const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) + + const module = new vm.SourceTextModule(str) + await module.link(() => {}) + await module.evaluate() + + res.send(`${module.namespace.result}`) + }, + vulnerability: 'CODE_INJECTION', + testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' + }) + + testThatRequestHasNoVulnerability(async (req, res) => { + const source = 'export const result = 1 + 2;' + const module = new vm.SourceTextModule(source) + await module.link(() => {}) + await module.evaluate() + + res.send(`${module.namespace.result}`) + }, 'CODE_INJECTION') }) - - testThatRequestHasNoVulnerability((req, res) => { - const result = vm.runInThisContext('1 + 2') - - res.send(`${result}`) - }, 'CODE_INJECTION') - }) + }) }) }) }) From 6dfe6ca7c66031e332864ee4d6570170d7f0a42b Mon Sep 17 00:00:00 2001 From: ishabi Date: Wed, 15 Jan 2025 17:32:45 +0100 Subject: [PATCH 4/6] add code injection integration test --- package.json | 2 +- ...-injection-analyzer.express.plugin.spec.js | 49 ------------ .../iast/code_injection.integration.spec.js | 78 +++++++++++++++++++ .../dd-trace/test/appsec/iast/resources/vm.js | 24 ++++++ 4 files changed, 103 insertions(+), 50 deletions(-) create mode 100644 packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/resources/vm.js diff --git a/package.json b/package.json index 6bb87d3ad5c..fedd38e7312 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "test": "SERVICES=* yarn services && mocha --expose-gc 'packages/dd-trace/test/setup/node.js' 'packages/*/test/**/*.spec.js'", "test:appsec": "mocha -r \"packages/dd-trace/test/setup/mocha.js\" --exclude \"packages/dd-trace/test/appsec/**/*.plugin.spec.js\" \"packages/dd-trace/test/appsec/**/*.spec.js\"", "test:appsec:ci": "nyc --no-clean --include \"packages/dd-trace/src/appsec/**/*.js\" --exclude \"packages/dd-trace/test/appsec/**/*.plugin.spec.js\" -- npm run test:appsec", - "test:appsec:plugins": "mocha -r \"packages/dd-trace/test/setup/mocha.js\" --experimental-vm-modules \"packages/dd-trace/test/appsec/**/*.@($(echo $PLUGINS)).plugin.spec.js\"", + "test:appsec:plugins": "mocha -r \"packages/dd-trace/test/setup/mocha.js\" \"packages/dd-trace/test/appsec/**/*.@($(echo $PLUGINS)).plugin.spec.js\"", "test:appsec:plugins:ci": "yarn services && nyc --no-clean --include \"packages/dd-trace/src/appsec/**/*.js\" -- npm run test:appsec:plugins", "test:debugger": "mocha -r 'packages/dd-trace/test/setup/mocha.js' 'packages/dd-trace/test/debugger/**/*.spec.js'", "test:debugger:ci": "nyc --no-clean --include 'packages/dd-trace/src/debugger/**/*.js' -- npm run test:debugger", diff --git a/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js index 840159fd63f..9b2fcf2b36c 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/code-injection-analyzer.express.plugin.spec.js @@ -373,55 +373,6 @@ describe('Code injection vulnerability', () => { }, 'CODE_INJECTION') }) }) - - describe('SourceTextModule class', () => { - prepareTestServerForIastInExpress('SourceTextModule in express', version, - (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => { - testThatRequestHasVulnerability({ - fn: async (req, res) => { - const module = new vm.SourceTextModule(req.query.script) - await module.link(() => {}) - await module.evaluate() - - res.send(`${module.namespace.result}`) - }, - vulnerability: 'CODE_INJECTION', - makeRequest: (done, config) => { - axios.get(`http://localhost:${config.port}/?script=export%20const%20result%20%3D%203%3B`) - .then(res => { - expect(res.data).to.equal(3) - }) - .catch(done) - } - }) - - testThatRequestHasVulnerability({ - fn: async (req, res) => { - const source = 'export const result = 1 + 2;' - const store = storage.getStore() - const iastContext = iastContextFunctions.getIastContext(store) - const str = newTaintedString(iastContext, source, 'param', SQL_ROW_VALUE) - - const module = new vm.SourceTextModule(str) - await module.link(() => {}) - await module.evaluate() - - res.send(`${module.namespace.result}`) - }, - vulnerability: 'CODE_INJECTION', - testDescription: 'Should detect CODE_INJECTION vulnerability with DB source' - }) - - testThatRequestHasNoVulnerability(async (req, res) => { - const source = 'export const result = 1 + 2;' - const module = new vm.SourceTextModule(source) - await module.link(() => {}) - await module.evaluate() - - res.send(`${module.namespace.result}`) - }, 'CODE_INJECTION') - }) - }) }) }) }) diff --git a/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js b/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js new file mode 100644 index 00000000000..e0858b22258 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js @@ -0,0 +1,78 @@ +'use strict' + +const { createSandbox, FakeAgent, spawnProc } = require('../../../../../integration-tests/helpers') +const getPort = require('get-port') +const path = require('path') +const Axios = require('axios') + +describe('IAST - code_injection - integration', () => { + let axios, sandbox, cwd, appPort, appFile, agent, proc + + before(async function () { + this.timeout(process.platform === 'win32' ? 90000 : 30000) + + sandbox = await createSandbox( + ['express'], + false, + [path.join(__dirname, 'resources')] + ) + + appPort = await getPort() + cwd = sandbox.folder + appFile = path.join(cwd, 'resources', 'vm.js') + + axios = Axios.create({ + baseURL: `http://localhost:${appPort}` + }) + }) + + after(async function () { + this.timeout(60000) + await sandbox.remove() + }) + + beforeEach(async () => { + agent = await new FakeAgent().start() + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + DD_TRACE_DEBUG: 'true', + APP_PORT: appPort, + DD_APPSEC_ENABLED: 'true', + DD_IAST_ENABLED: 'true', + DD_IAST_REQUEST_SAMPLING: '100' + }, + execArgv: ['--experimental-vm-modules'] + }) + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + async function testVulnerabilityRepoting (url) { + await axios.get(url) + + return agent.assertMessageReceived(({ headers, payload }) => { + expect(payload[0][0].metrics['_dd.iast.enabled']).to.be.equal(1) + expect(payload[0][0].meta).to.have.property('_dd.iast.json') + const vulnerabilitiesTrace = JSON.parse(payload[0][0].meta['_dd.iast.json']) + expect(vulnerabilitiesTrace).to.not.be.null + const vulnerabilities = new Set() + + vulnerabilitiesTrace.vulnerabilities.forEach(v => { + vulnerabilities.add(v.type) + }) + + expect(vulnerabilities.has('CODE_INJECTION')).to.be.true + }) + } + + describe('SourceTextModule', () => { + it('should report Code injection vulnerability', async () => { + await testVulnerabilityRepoting('/vm/SourceTextModule?script=export%20const%20result%20%3D%203%3B') + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/resources/vm.js b/packages/dd-trace/test/appsec/iast/resources/vm.js new file mode 100644 index 00000000000..3719d445c43 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/resources/vm.js @@ -0,0 +1,24 @@ +'use strict' + +const tracer = require('dd-trace') +tracer.init({ + flushInterval: 1 +}) + +const express = require('express') +const vm = require('node:vm') + +const app = express() +const port = process.env.APP_PORT || 3000 + +app.get('/vm/SourceTextModule', async (req, res) => { + const module = new vm.SourceTextModule(req.query.script) + await module.link(() => {}) + await module.evaluate() + + res.end('OK') +}) + +app.listen(port, () => { + process.send({ port }) +}) From 1fbb006f3dcaafedc39caf4d4ff943d9b18d18e3 Mon Sep 17 00:00:00 2001 From: ishabi Date: Thu, 16 Jan 2025 11:35:32 +0100 Subject: [PATCH 5/6] instrument SourceTextModule only if it's enabled --- packages/datadog-instrumentations/src/vm.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/datadog-instrumentations/src/vm.js b/packages/datadog-instrumentations/src/vm.js index 7bca1787481..ff9f8bfbffb 100644 --- a/packages/datadog-instrumentations/src/vm.js +++ b/packages/datadog-instrumentations/src/vm.js @@ -18,12 +18,14 @@ addHook({ name: names }, function (vm) { } } - vm.SourceTextModule = class extends vm.SourceTextModule { - constructor (sourceText) { - super(...arguments) - - if (sourceTextModuleStartChannel.hasSubscribers && sourceText) { - sourceTextModuleStartChannel.publish({ sourceText }) + if (vm.SourceTextModule && typeof vm.SourceTextModule === 'function') { + vm.SourceTextModule = class extends vm.SourceTextModule { + constructor (sourceText) { + super(...arguments) + + if (sourceTextModuleStartChannel.hasSubscribers && sourceText) { + sourceTextModuleStartChannel.publish({ sourceText }) + } } } } From f5b3e2a8d5520e1ccdb6a1610e480560906926b1 Mon Sep 17 00:00:00 2001 From: ishabi Date: Fri, 17 Jan 2025 10:44:06 +0100 Subject: [PATCH 6/6] unify channel arguments --- packages/datadog-instrumentations/src/vm.js | 6 +++--- .../src/appsec/iast/analyzers/code-injection-analyzer.js | 2 +- .../test/appsec/iast/code_injection.integration.spec.js | 2 -- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/datadog-instrumentations/src/vm.js b/packages/datadog-instrumentations/src/vm.js index ff9f8bfbffb..9df229556fa 100644 --- a/packages/datadog-instrumentations/src/vm.js +++ b/packages/datadog-instrumentations/src/vm.js @@ -20,11 +20,11 @@ addHook({ name: names }, function (vm) { if (vm.SourceTextModule && typeof vm.SourceTextModule === 'function') { vm.SourceTextModule = class extends vm.SourceTextModule { - constructor (sourceText) { + constructor (code) { super(...arguments) - if (sourceTextModuleStartChannel.hasSubscribers && sourceText) { - sourceTextModuleStartChannel.publish({ sourceText }) + if (sourceTextModuleStartChannel.hasSubscribers && code) { + sourceTextModuleStartChannel.publish({ code }) } } } diff --git a/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js index fe36d83482b..6c60aad4d22 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/code-injection-analyzer.js @@ -11,7 +11,7 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer { onConfigure () { this.addSub('datadog:eval:call', ({ script }) => this.analyze(script)) this.addSub('datadog:vm:run-script:start', ({ code }) => this.analyze(code)) - this.addSub('datadog:vm:source-text-module:start', ({ sourceText }) => this.analyze(sourceText)) + this.addSub('datadog:vm:source-text-module:start', ({ code }) => this.analyze(code)) } _areRangesVulnerable () { diff --git a/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js b/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js index e0858b22258..60342c930c9 100644 --- a/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js +++ b/packages/dd-trace/test/appsec/iast/code_injection.integration.spec.js @@ -37,9 +37,7 @@ describe('IAST - code_injection - integration', () => { cwd, env: { DD_TRACE_AGENT_PORT: agent.port, - DD_TRACE_DEBUG: 'true', APP_PORT: appPort, - DD_APPSEC_ENABLED: 'true', DD_IAST_ENABLED: 'true', DD_IAST_REQUEST_SAMPLING: '100' },