From fd1dd7e1508aeebb6809a0442861b2e0e37166d9 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 12 Feb 2025 17:28:47 +0100 Subject: [PATCH] [asm] IAST security controls (#5117) * Security controls parser and secure marks for vulnerabilities * Use new NOSQL_MONGODB_INJECTION_MARK in nosql-injection-mongodb-analyzer * Config * first hooks * wrap object properties and more tests * Use dd-trace:moduleLoad(Start|End) channels * iterate object strings and more tests * fix parameter index, include createNewTainted flag and do not use PluginManager in the tests * Fix parameter index and include a test with incorrect index * Avoid to hook multiple times the same module and config tests * sql_injection_mark test * vulnerable ranges tests * fix windows paths * Upgrade taint-tracking to 3.3.0 * Fix * secure mark * add createNewTainted flag to addSecureMark * Use existing _isRangeSecure * supressed vulnerabilities metric * increment supressed vulnerability metric * typo * handle esm default export and filenames starting with file:// * esm integration tests * clean up * secure-marks tests * fix secure-marks generator test * fix config test * empty * check for repeated marks * Update packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js Co-authored-by: Ugaitz Urien * Update packages/dd-trace/src/appsec/iast/security-controls/index.js Co-authored-by: Ugaitz Urien * Update packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js Co-authored-by: Ugaitz Urien * some suggestions * move _isRangeSecure to InjectionAnalyzer * Add programatically config option * index.d.ts * StoredInjectionAnalyzer * Update packages/dd-trace/test/appsec/iast/analyzers/command-injection-analyzer.spec.js Co-authored-by: ishabi * store control keys to avoid recreating the array * check visited before iterating * test suggestions * Update packages/dd-trace/src/appsec/iast/security-controls/parser.js Co-authored-by: Ilyas Shabi * lint * ritm test * clean up * Reject security control with non numeric parameters * fix parameter 0 * Update integration-tests/appsec/iast.esm-security-controls.spec.js Co-authored-by: Ugaitz Urien * suggestions * use legacy store * fix test * fix test * fix test --------- Co-authored-by: Ugaitz Urien Co-authored-by: ishabi --- index.d.ts | 5 + .../appsec/esm-security-controls/index.mjs | 69 ++++ .../sanitizer-default.mjs | 7 + .../esm-security-controls/sanitizer.mjs | 5 + .../esm-security-controls/validator.mjs | 9 + .../appsec/iast.esm-security-controls.spec.js | 126 ++++++ package.json | 2 +- .../iast/analyzers/code-injection-analyzer.js | 8 +- .../iast/analyzers/injection-analyzer.js | 16 +- .../nosql-injection-mongodb-analyzer.js | 35 +- .../iast/analyzers/sql-injection-analyzer.js | 8 +- .../analyzers/stored-injection-analyzer.js | 11 + .../analyzers/template-injection-analyzer.js | 8 +- .../iast/analyzers/vulnerability-analyzer.js | 14 + packages/dd-trace/src/appsec/iast/index.js | 2 + .../appsec/iast/security-controls/index.js | 187 +++++++++ .../appsec/iast/security-controls/parser.js | 96 +++++ .../appsec/iast/taint-tracking/operations.js | 4 +- .../taint-tracking/secure-marks-generator.js | 2 +- .../iast/taint-tracking/secure-marks.js | 28 ++ .../src/appsec/iast/telemetry/iast-metric.js | 5 + packages/dd-trace/src/appsec/iast/utils.js | 24 ++ packages/dd-trace/src/config.js | 6 +- packages/dd-trace/src/ritm.js | 3 +- .../iast/analyzers/injection-analyzer.spec.js | 100 +++++ .../nosql-injection-mongodb-analyzer.spec.js | 13 +- .../analyzers/sql-injection-analyzer.spec.js | 67 +++- .../iast/security-controls/index.spec.js | 361 ++++++++++++++++++ .../iast/security-controls/parser.spec.js | 288 ++++++++++++++ .../resources/custom_input_validator.js | 14 + .../node_modules/sanitizer/index.js | 13 + .../resources/node_modules/sanitizer/index.js | 13 + .../security-controls/resources/sanitizer.js | 18 + .../resources/sanitizer_default.js | 7 + .../secure-marks-generator.spec.js | 2 +- .../iast/taint-tracking/secure-marks.spec.js | 34 ++ packages/dd-trace/test/config.spec.js | 24 +- .../test/ritm-tests/module-default.js | 5 + packages/dd-trace/test/ritm.spec.js | 18 + yarn.lock | 8 +- 40 files changed, 1589 insertions(+), 76 deletions(-) create mode 100644 integration-tests/appsec/esm-security-controls/index.mjs create mode 100644 integration-tests/appsec/esm-security-controls/sanitizer-default.mjs create mode 100644 integration-tests/appsec/esm-security-controls/sanitizer.mjs create mode 100644 integration-tests/appsec/esm-security-controls/validator.mjs create mode 100644 integration-tests/appsec/iast.esm-security-controls.spec.js create mode 100644 packages/dd-trace/src/appsec/iast/analyzers/stored-injection-analyzer.js create mode 100644 packages/dd-trace/src/appsec/iast/security-controls/index.js create mode 100644 packages/dd-trace/src/appsec/iast/security-controls/parser.js create mode 100644 packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js create mode 100644 packages/dd-trace/src/appsec/iast/utils.js create mode 100644 packages/dd-trace/test/appsec/iast/analyzers/injection-analyzer.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/index.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/parser.spec.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/resources/custom_input_validator.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/anotherlib/node_modules/sanitizer/index.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/sanitizer/index.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer.js create mode 100644 packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer_default.js create mode 100644 packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks.spec.js create mode 100644 packages/dd-trace/test/ritm-tests/module-default.js diff --git a/index.d.ts b/index.d.ts index 7b5a345ddfd..771988ce788 100644 --- a/index.d.ts +++ b/index.d.ts @@ -2226,6 +2226,11 @@ declare namespace tracer { */ redactionValuePattern?: string, + /** + * Allows to enable security controls. + */ + securityControlsConfiguration?: string, + /** * Specifies the verbosity of the sent telemetry. Default 'INFORMATION' */ diff --git a/integration-tests/appsec/esm-security-controls/index.mjs b/integration-tests/appsec/esm-security-controls/index.mjs new file mode 100644 index 00000000000..c9bcadb017c --- /dev/null +++ b/integration-tests/appsec/esm-security-controls/index.mjs @@ -0,0 +1,69 @@ +'use strict' + +import childProcess from 'node:child_process' +import express from 'express' +import { sanitize } from './sanitizer.mjs' +import sanitizeDefault from './sanitizer-default.mjs' +import { validate, validateNotConfigured } from './validator.mjs' + +const app = express() +const port = process.env.APP_PORT || 3000 + +app.get('/cmdi-s-secure', (req, res) => { + const command = sanitize(req.query.command) + try { + childProcess.execSync(command) + } catch (e) { + // ignore + } + + res.end() +}) + +app.get('/cmdi-s-secure-comparison', (req, res) => { + const command = sanitize(req.query.command) + try { + childProcess.execSync(command) + } catch (e) { + // ignore + } + + try { + childProcess.execSync(req.query.command) + } catch (e) { + // ignore + } + + res.end() +}) + +app.get('/cmdi-s-secure-default', (req, res) => { + const command = sanitizeDefault(req.query.command) + try { + childProcess.execSync(command) + } catch (e) { + // ignore + } + + res.end() +}) + +app.get('/cmdi-iv-insecure', (req, res) => { + if (validateNotConfigured(req.query.command)) { + childProcess.execSync(req.query.command) + } + + res.end() +}) + +app.get('/cmdi-iv-secure', (req, res) => { + if (validate(req.query.command)) { + childProcess.execSync(req.query.command) + } + + res.end() +}) + +app.listen(port, () => { + process.send({ port }) +}) diff --git a/integration-tests/appsec/esm-security-controls/sanitizer-default.mjs b/integration-tests/appsec/esm-security-controls/sanitizer-default.mjs new file mode 100644 index 00000000000..6e580f450c5 --- /dev/null +++ b/integration-tests/appsec/esm-security-controls/sanitizer-default.mjs @@ -0,0 +1,7 @@ +'use strict' + +function sanitizeDefault (input) { + return input +} + +export default sanitizeDefault diff --git a/integration-tests/appsec/esm-security-controls/sanitizer.mjs b/integration-tests/appsec/esm-security-controls/sanitizer.mjs new file mode 100644 index 00000000000..4529126061d --- /dev/null +++ b/integration-tests/appsec/esm-security-controls/sanitizer.mjs @@ -0,0 +1,5 @@ +'use strict' + +export function sanitize (input) { + return input +} diff --git a/integration-tests/appsec/esm-security-controls/validator.mjs b/integration-tests/appsec/esm-security-controls/validator.mjs new file mode 100644 index 00000000000..3542aa8d17c --- /dev/null +++ b/integration-tests/appsec/esm-security-controls/validator.mjs @@ -0,0 +1,9 @@ +'use strict' + +export function validate (input) { + return true +} + +export function validateNotConfigured (input) { + return true +} diff --git a/integration-tests/appsec/iast.esm-security-controls.spec.js b/integration-tests/appsec/iast.esm-security-controls.spec.js new file mode 100644 index 00000000000..457987ac99a --- /dev/null +++ b/integration-tests/appsec/iast.esm-security-controls.spec.js @@ -0,0 +1,126 @@ +'use strict' + +const { createSandbox, spawnProc, FakeAgent } = require('../helpers') +const path = require('path') +const getPort = require('get-port') +const Axios = require('axios') +const { assert } = require('chai') + +describe('ESM Security controls', () => { + let axios, sandbox, cwd, appPort, appFile, agent, proc + + before(async function () { + this.timeout(process.platform === 'win32' ? 90000 : 30000) + sandbox = await createSandbox(['express']) + appPort = await getPort() + cwd = sandbox.folder + appFile = path.join(cwd, 'appsec', 'esm-security-controls', 'index.mjs') + + axios = Axios.create({ + baseURL: `http://localhost:${appPort}` + }) + }) + + after(async function () { + await sandbox.remove() + }) + + const nodeOptions = '--import dd-trace/initialize.mjs' + + beforeEach(async () => { + agent = await new FakeAgent().start() + + proc = await spawnProc(appFile, { + cwd, + env: { + DD_TRACE_AGENT_PORT: agent.port, + APP_PORT: appPort, + DD_IAST_ENABLED: 'true', + DD_IAST_REQUEST_SAMPLING: '100', + // eslint-disable-next-line no-multi-str + DD_IAST_SECURITY_CONTROLS_CONFIGURATION: '\ + SANITIZER:COMMAND_INJECTION:appsec/esm-security-controls/sanitizer.mjs:sanitize;\ + SANITIZER:COMMAND_INJECTION:appsec/esm-security-controls/sanitizer-default.mjs;\ + INPUT_VALIDATOR:*:appsec/esm-security-controls/validator.mjs:validate', + NODE_OPTIONS: nodeOptions + } + }) + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + it('test endpoint with iv not configured does have COMMAND_INJECTION vulnerability', async function () { + await axios.get('/cmdi-iv-insecure?command=ls -la') + + await agent.assertMessageReceived(({ payload }) => { + const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request')) + spans.forEach(span => { + assert.property(span.meta, '_dd.iast.json') + assert.include(span.meta['_dd.iast.json'], '"COMMAND_INJECTION"') + }) + }, null, 1, true) + }) + + it('test endpoint sanitizer does not have COMMAND_INJECTION vulnerability', async () => { + await axios.get('/cmdi-s-secure?command=ls -la') + + await agent.assertMessageReceived(({ payload }) => { + const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request')) + spans.forEach(span => { + assert.notProperty(span.meta, '_dd.iast.json') + assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection') + }) + }, null, 1, true) + }) + + it('test endpoint with default sanitizer does not have COMMAND_INJECTION vulnerability', async () => { + await axios.get('/cmdi-s-secure-default?command=ls -la') + + await agent.assertMessageReceived(({ payload }) => { + const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request')) + spans.forEach(span => { + assert.notProperty(span.meta, '_dd.iast.json') + assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection') + }) + }, null, 1, true) + }) + + it('test endpoint with default sanitizer does have COMMAND_INJECTION with original tainted', async () => { + await axios.get('/cmdi-s-secure-comparison?command=ls -la') + + await agent.assertMessageReceived(({ payload }) => { + const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request')) + spans.forEach(span => { + assert.property(span.meta, '_dd.iast.json') + assert.include(span.meta['_dd.iast.json'], '"COMMAND_INJECTION"') + }) + }, null, 1, true) + }) + + it('test endpoint with default sanitizer does have COMMAND_INJECTION vulnerability', async () => { + await axios.get('/cmdi-s-secure-default?command=ls -la') + + await agent.assertMessageReceived(({ payload }) => { + const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request')) + spans.forEach(span => { + assert.notProperty(span.meta, '_dd.iast.json') + assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection') + }) + }, null, 1, true) + }) + + it('test endpoint with iv does not have COMMAND_INJECTION vulnerability', async () => { + await axios.get('/cmdi-iv-secure?command=ls -la') + + await agent.assertMessageReceived(({ payload }) => { + const spans = payload.flatMap(p => p.filter(span => span.name === 'express.request')) + spans.forEach(span => { + assert.notProperty(span.meta, '_dd.iast.json') + assert.property(span.metrics, '_dd.iast.telemetry.suppressed.vulnerabilities.command_injection') + }) + }, null, 1, true) + }) +}) diff --git a/package.json b/package.json index 1c9b2c400a0..c0d9526f160 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,7 @@ "@datadog/libdatadog": "^0.4.0", "@datadog/native-appsec": "8.4.0", "@datadog/native-iast-rewriter": "2.8.0", - "@datadog/native-iast-taint-tracking": "3.2.0", + "@datadog/native-iast-taint-tracking": "3.3.0", "@datadog/native-metrics": "^3.1.0", "@datadog/pprof": "5.5.1", "@datadog/sketches-js": "^2.1.0", 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 03582a3064a..60d1f81e541 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 @@ -1,12 +1,12 @@ 'use strict' -const InjectionAnalyzer = require('./injection-analyzer') const { CODE_INJECTION } = require('../vulnerabilities') +const StoredInjectionAnalyzer = require('./stored-injection-analyzer') const { INSTRUMENTED_SINK } = require('../telemetry/iast-metric') const { storage } = require('../../../../../datadog-core') const { getIastContext } = require('../iast-context') -class CodeInjectionAnalyzer extends InjectionAnalyzer { +class CodeInjectionAnalyzer extends StoredInjectionAnalyzer { constructor () { super(CODE_INJECTION) this.evalInstrumentedInc = false @@ -31,10 +31,6 @@ class CodeInjectionAnalyzer extends InjectionAnalyzer { this.addSub('datadog:vm:run-script:start', ({ code }) => this.analyze(code)) this.addSub('datadog:vm:source-text-module:start', ({ code }) => this.analyze(code)) } - - _areRangesVulnerable () { - return true - } } module.exports = new CodeInjectionAnalyzer() diff --git a/packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js index f0d42bf95ae..a0b47c7dc3a 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/injection-analyzer.js @@ -5,8 +5,13 @@ const { SQL_ROW_VALUE } = require('../taint-tracking/source-types') class InjectionAnalyzer extends Analyzer { _isVulnerable (value, iastContext) { - const ranges = value && getRanges(iastContext, value) + let ranges = value && getRanges(iastContext, value) if (ranges?.length > 0) { + ranges = this._filterSecureRanges(ranges) + if (!ranges?.length) { + this._incrementSuppressedMetric(iastContext) + } + return this._areRangesVulnerable(ranges) } @@ -21,6 +26,15 @@ class InjectionAnalyzer extends Analyzer { _areRangesVulnerable (ranges) { return ranges?.some(range => range.iinfo.type !== SQL_ROW_VALUE) } + + _filterSecureRanges (ranges) { + return ranges?.filter(range => !this._isRangeSecure(range)) + } + + _isRangeSecure (range) { + const { secureMarks } = range + return (secureMarks & this._secureMark) === this._secureMark + } } module.exports = InjectionAnalyzer diff --git a/packages/dd-trace/src/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.js index b73c069a5f0..78617c6f047 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.js @@ -4,29 +4,13 @@ const InjectionAnalyzer = require('./injection-analyzer') const { NOSQL_MONGODB_INJECTION } = require('../vulnerabilities') const { getRanges, addSecureMark } = require('../taint-tracking/operations') const { getNodeModulesPaths } = require('../path-line') -const { getNextSecureMark } = require('../taint-tracking/secure-marks-generator') const { storage } = require('../../../../../datadog-core') const { getIastContext } = require('../iast-context') const { HTTP_REQUEST_PARAMETER, HTTP_REQUEST_BODY } = require('../taint-tracking/source-types') const EXCLUDED_PATHS_FROM_STACK = getNodeModulesPaths('mongodb', 'mongoose', 'mquery') -const MONGODB_NOSQL_SECURE_MARK = getNextSecureMark() - -function iterateObjectStrings (target, fn, levelKeys = [], depth = 20, visited = new Set()) { - if (target !== null && typeof target === 'object') { - Object.keys(target).forEach((key) => { - const nextLevelKeys = [...levelKeys, key] - const val = target[key] - - if (typeof val === 'string') { - fn(val, nextLevelKeys, target, key) - } else if (depth > 0 && !visited.has(val)) { - iterateObjectStrings(val, fn, nextLevelKeys, depth - 1, visited) - visited.add(val) - } - }) - } -} +const { NOSQL_MONGODB_INJECTION_MARK } = require('../taint-tracking/secure-marks') +const { iterateObjectStrings } = require('../utils') class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer { constructor () { @@ -88,7 +72,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer { const currentLevelKey = levelKeys[i] if (i === levelsLength - 1) { - parentObj[currentLevelKey] = addSecureMark(iastContext, value, MONGODB_NOSQL_SECURE_MARK) + parentObj[currentLevelKey] = addSecureMark(iastContext, value, NOSQL_MONGODB_INJECTION_MARK) } else { parentObj = parentObj[currentLevelKey] } @@ -106,7 +90,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer { if (iastContext) { // do nothing if we are not in an iast request iterateObjectStrings(sanitizedObject, function (value, levelKeys, parent, lastKey) { try { - parent[lastKey] = addSecureMark(iastContext, value, MONGODB_NOSQL_SECURE_MARK) + parent[lastKey] = addSecureMark(iastContext, value, NOSQL_MONGODB_INJECTION_MARK) } catch { // if it is a readonly property, do nothing } @@ -121,8 +105,7 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer { _isVulnerableRange (range) { const rangeType = range?.iinfo?.type - const isVulnerableType = rangeType === HTTP_REQUEST_PARAMETER || rangeType === HTTP_REQUEST_BODY - return isVulnerableType && (range.secureMarks & MONGODB_NOSQL_SECURE_MARK) !== MONGODB_NOSQL_SECURE_MARK + return rangeType === HTTP_REQUEST_PARAMETER || rangeType === HTTP_REQUEST_BODY } _isVulnerable (value, iastContext) { @@ -137,10 +120,15 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer { const allRanges = [] iterateObjectStrings(value.filter, (val, nextLevelKeys) => { - const ranges = getRanges(iastContext, val) + let ranges = getRanges(iastContext, val) if (ranges?.length) { const filteredRanges = [] + ranges = this._filterSecureRanges(ranges) + if (!ranges.length) { + this._incrementSuppressedMetric(iastContext) + } + for (const range of ranges) { if (this._isVulnerableRange(range)) { isVulnerable = true @@ -175,4 +163,3 @@ class NosqlInjectionMongodbAnalyzer extends InjectionAnalyzer { } module.exports = new NosqlInjectionMongodbAnalyzer() -module.exports.MONGODB_NOSQL_SECURE_MARK = MONGODB_NOSQL_SECURE_MARK diff --git a/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js index 2e6415e36a0..50e7b5966bc 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/sql-injection-analyzer.js @@ -1,14 +1,14 @@ 'use strict' -const InjectionAnalyzer = require('./injection-analyzer') const { SQL_INJECTION } = require('../vulnerabilities') const { getRanges } = require('../taint-tracking/operations') const { storage } = require('../../../../../datadog-core') const { getNodeModulesPaths } = require('../path-line') +const StoredInjectionAnalyzer = require('./stored-injection-analyzer') const EXCLUDED_PATHS = getNodeModulesPaths('mysql', 'mysql2', 'sequelize', 'pg-pool', 'knex') -class SqlInjectionAnalyzer extends InjectionAnalyzer { +class SqlInjectionAnalyzer extends StoredInjectionAnalyzer { constructor () { super(SQL_INJECTION) } @@ -82,10 +82,6 @@ class SqlInjectionAnalyzer extends InjectionAnalyzer { return knexDialect.toUpperCase() } } - - _areRangesVulnerable () { - return true - } } module.exports = new SqlInjectionAnalyzer() diff --git a/packages/dd-trace/src/appsec/iast/analyzers/stored-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/stored-injection-analyzer.js new file mode 100644 index 00000000000..b2cd6e931ad --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/analyzers/stored-injection-analyzer.js @@ -0,0 +1,11 @@ +'use strict' + +const InjectionAnalyzer = require('./injection-analyzer') + +class StoredInjectionAnalyzer extends InjectionAnalyzer { + _areRangesVulnerable (ranges) { + return ranges?.length > 0 + } +} + +module.exports = StoredInjectionAnalyzer diff --git a/packages/dd-trace/src/appsec/iast/analyzers/template-injection-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/template-injection-analyzer.js index 8a5af919b2d..eff272cfb3f 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/template-injection-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/template-injection-analyzer.js @@ -1,9 +1,9 @@ 'use strict' -const InjectionAnalyzer = require('./injection-analyzer') const { TEMPLATE_INJECTION } = require('../vulnerabilities') +const StoredInjectionAnalyzer = require('./stored-injection-analyzer') -class TemplateInjectionAnalyzer extends InjectionAnalyzer { +class TemplateInjectionAnalyzer extends StoredInjectionAnalyzer { constructor () { super(TEMPLATE_INJECTION) } @@ -13,10 +13,6 @@ class TemplateInjectionAnalyzer extends InjectionAnalyzer { this.addSub('datadog:handlebars:register-partial:start', ({ partial }) => this.analyze(partial)) this.addSub('datadog:pug:compile:start', ({ source }) => this.analyze(source)) } - - _areRangesVulnerable () { - return true - } } module.exports = new TemplateInjectionAnalyzer() diff --git a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js index 2c17e9dcdb2..1cfff5c2b22 100644 --- a/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js +++ b/packages/dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer.js @@ -10,11 +10,14 @@ const { getVulnerabilityCallSiteFrames, replaceCallSiteFromSourceMap } = require('../vulnerability-reporter') +const { getMarkFromVulnerabilityType } = require('../taint-tracking/secure-marks') +const { SUPPRESSED_VULNERABILITIES } = require('../telemetry/iast-metric') class Analyzer extends SinkIastPlugin { constructor (type) { super() this._type = type + this._secureMark = getMarkFromVulnerabilityType(type) } _isVulnerable (value, context) { @@ -155,6 +158,17 @@ class Analyzer extends SinkIastPlugin { return hash } + _getSuppressedMetricTag () { + if (!this._suppressedMetricTag) { + this._suppressedMetricTag = SUPPRESSED_VULNERABILITIES.formatTags(this._type)[0] + } + return this._suppressedMetricTag + } + + _incrementSuppressedMetric (iastContext) { + SUPPRESSED_VULNERABILITIES.inc(iastContext, this._getSuppressedMetricTag()) + } + addSub (iastSubOrChannelName, handler) { const iastSub = typeof iastSubOrChannelName === 'string' ? { channelName: iastSubOrChannelName } diff --git a/packages/dd-trace/src/appsec/iast/index.js b/packages/dd-trace/src/appsec/iast/index.js index f185f315030..1af7411b218 100644 --- a/packages/dd-trace/src/appsec/iast/index.js +++ b/packages/dd-trace/src/appsec/iast/index.js @@ -15,6 +15,7 @@ const { const { IAST_ENABLED_TAG_KEY } = require('./tags') const iastTelemetry = require('./telemetry') const { enable: enableFsPlugin, disable: disableFsPlugin, IAST_MODULE } = require('../rasp/fs-plugin') +const securityControls = require('./security-controls') // TODO Change to `apm:http:server:request:[start|close]` when the subscription // order of the callbacks can be enforce @@ -35,6 +36,7 @@ function enable (config, _tracer) { requestClose.subscribe(onIncomingHttpRequestEnd) overheadController.configure(config.iast) overheadController.startGlobalContext() + securityControls.configure(config.iast) vulnerabilityReporter.start(config, _tracer) isEnabled = true diff --git a/packages/dd-trace/src/appsec/iast/security-controls/index.js b/packages/dd-trace/src/appsec/iast/security-controls/index.js new file mode 100644 index 00000000000..9c12805ab1c --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/security-controls/index.js @@ -0,0 +1,187 @@ +'use strict' + +const path = require('path') +const dc = require('dc-polyfill') +const { storage } = require('../../../../../datadog-core') +const shimmer = require('../../../../../datadog-shimmer') +const log = require('../../../log') +const { parse, SANITIZER_TYPE } = require('./parser') +const TaintTrackingOperations = require('../taint-tracking/operations') +const { getIastContext } = require('../iast-context') +const { iterateObjectStrings } = require('../utils') + +// esm +const moduleLoadStartChannel = dc.channel('dd-trace:moduleLoadStart') + +// cjs +const moduleLoadEndChannel = dc.channel('dd-trace:moduleLoadEnd') + +let controls +let controlsKeys +let hooks + +function configure (iastConfig) { + if (!iastConfig?.securityControlsConfiguration) return + + try { + controls = parse(iastConfig.securityControlsConfiguration) + if (controls?.size > 0) { + hooks = new WeakSet() + controlsKeys = [...controls.keys()] + + moduleLoadStartChannel.subscribe(onModuleLoaded) + moduleLoadEndChannel.subscribe(onModuleLoaded) + } + } catch (e) { + log.error('[ASM] Error configuring IAST Security Controls', e) + } +} + +function onModuleLoaded (payload) { + if (!payload?.module || hooks?.has(payload.module)) return + + const { filename, module } = payload + + const controlsByFile = getControls(filename) + if (controlsByFile) { + const hook = hookModule(filename, module, controlsByFile) + payload.module = hook + hooks.add(hook) + } +} + +function getControls (filename) { + if (filename.startsWith('file://')) { + filename = filename.substring(7) + } + + let key = path.isAbsolute(filename) ? path.relative(process.cwd(), filename) : filename + key = key.replaceAll(path.sep, path.posix.sep) + + if (key.includes('node_modules')) { + key = controlsKeys.find(file => key.endsWith(file)) + } + + return controls.get(key) +} + +function hookModule (filename, module, controlsByFile) { + try { + controlsByFile.forEach(({ type, method, parameters, secureMarks }) => { + const { target, parent, methodName } = resolve(method, module) + if (!target) { + log.error('[ASM] Unable to resolve IAST security control %s:%s', filename, method) + return + } + + let wrapper + if (type === SANITIZER_TYPE) { + wrapper = wrapSanitizer(target, secureMarks) + } else { + wrapper = wrapInputValidator(target, parameters, secureMarks) + } + + if (methodName) { + parent[methodName] = wrapper + } else { + module = wrapper + } + }) + } catch (e) { + log.error('[ASM] Error initializing IAST security control for %', filename, e) + } + + return module +} + +function resolve (path, obj, separator = '.') { + if (!path) { + // esm module with default export + if (obj?.default) { + return { target: obj.default, parent: obj, methodName: 'default' } + } else { + return { target: obj, parent: obj } + } + } + + const properties = path.split(separator) + + let parent + let methodName + const target = properties.reduce((prev, curr) => { + parent = prev + methodName = curr + return prev?.[curr] + }, obj) + + return { target, parent, methodName } +} + +function wrapSanitizer (target, secureMarks) { + return shimmer.wrapFunction(target, orig => function () { + const result = orig.apply(this, arguments) + + try { + return addSecureMarks(result, secureMarks) + } catch (e) { + log.error('[ASM] Error adding Secure mark for sanitizer', e) + } + + return result + }) +} + +function wrapInputValidator (target, parameters, secureMarks) { + const allParameters = !parameters?.length + + return shimmer.wrapFunction(target, orig => function () { + try { + [...arguments].forEach((arg, index) => { + if (allParameters || parameters.includes(index)) { + addSecureMarks(arg, secureMarks, false) + } + }) + } catch (e) { + log.error('[ASM] Error adding Secure mark for input validator', e) + } + + return orig.apply(this, arguments) + }) +} + +function addSecureMarks (value, secureMarks, createNewTainted = true) { + if (!value) return + + const store = storage('legacy').getStore() + const iastContext = getIastContext(store) + + if (typeof value === 'string') { + return TaintTrackingOperations.addSecureMark(iastContext, value, secureMarks, createNewTainted) + } else { + iterateObjectStrings(value, (value, levelKeys, parent, lastKey) => { + try { + const securedTainted = TaintTrackingOperations.addSecureMark(iastContext, value, secureMarks, createNewTainted) + if (createNewTainted) { + parent[lastKey] = securedTainted + } + } catch (e) { + // if it is a readonly property, do nothing + } + }) + return value + } +} + +function disable () { + if (moduleLoadStartChannel.hasSubscribers) moduleLoadStartChannel.unsubscribe(onModuleLoaded) + if (moduleLoadEndChannel.hasSubscribers) moduleLoadEndChannel.unsubscribe(onModuleLoaded) + + controls = undefined + controlsKeys = undefined + hooks = undefined +} + +module.exports = { + configure, + disable +} diff --git a/packages/dd-trace/src/appsec/iast/security-controls/parser.js b/packages/dd-trace/src/appsec/iast/security-controls/parser.js new file mode 100644 index 00000000000..aef3d6627bb --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/security-controls/parser.js @@ -0,0 +1,96 @@ +'use strict' + +const log = require('../../../log') +const { getMarkFromVulnerabilityType, CUSTOM_SECURE_MARK } = require('../taint-tracking/secure-marks') + +const SECURITY_CONTROL_DELIMITER = ';' +const SECURITY_CONTROL_FIELD_DELIMITER = ':' +const SECURITY_CONTROL_ELEMENT_DELIMITER = ',' + +const INPUT_VALIDATOR_TYPE = 'INPUT_VALIDATOR' +const SANITIZER_TYPE = 'SANITIZER' + +const validTypes = [INPUT_VALIDATOR_TYPE, SANITIZER_TYPE] + +function parse (securityControlsConfiguration) { + const controls = new Map() + + securityControlsConfiguration?.replace(/[\r\n\t\v\f]*/g, '') + .split(SECURITY_CONTROL_DELIMITER) + .map(parseControl) + .filter(control => !!control) + .forEach(control => { + if (!controls.has(control.file)) { + controls.set(control.file, []) + } + controls.get(control.file).push(control) + }) + + return controls +} + +function parseControl (control) { + if (!control) return + + const fields = control.split(SECURITY_CONTROL_FIELD_DELIMITER) + + if (fields.length < 3 || fields.length > 5) { + log.warn('[ASM] Security control configuration is invalid: %s', control) + return + } + + let [type, marks, file, method, parameters] = fields + + type = type.trim().toUpperCase() + if (!validTypes.includes(type)) { + log.warn('[ASM] Invalid security control type: %s', type) + return + } + + let secureMarks = CUSTOM_SECURE_MARK + getSecureMarks(marks).forEach(mark => { secureMarks |= mark }) + if (secureMarks === CUSTOM_SECURE_MARK) { + log.warn('[ASM] Invalid security control mark: %s', marks) + return + } + + file = file?.trim() + + method = method?.trim() + + try { + parameters = getParameters(parameters) + } catch (e) { + log.warn('[ASM] Invalid non-numeric security control parameter %s', parameters) + return + } + + return { type, secureMarks, file, method, parameters } +} + +function getSecureMarks (marks) { + return marks?.split(SECURITY_CONTROL_ELEMENT_DELIMITER) + .map(getMarkFromVulnerabilityType) + .filter(mark => !!mark) +} + +function getParameters (parameters) { + return parameters?.split(SECURITY_CONTROL_ELEMENT_DELIMITER) + .map(param => { + const parsedParam = parseInt(param, 10) + + // discard the securityControl if there is an incorrect parameter + if (isNaN(parsedParam)) { + throw new Error('Invalid non-numeric security control parameter') + } + + return parsedParam + }) +} + +module.exports = { + parse, + + INPUT_VALIDATOR_TYPE, + SANITIZER_TYPE +} diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js b/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js index ce530b03702..815f430e6c6 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/operations.js @@ -84,10 +84,10 @@ function getRanges (iastContext, string) { return result } -function addSecureMark (iastContext, string, mark) { +function addSecureMark (iastContext, string, mark, createNewTainted = true) { const transactionId = iastContext?.[IAST_TRANSACTION_ID] if (transactionId) { - return TaintedUtils.addSecureMarksToTaintedString(transactionId, string, mark) + return TaintedUtils.addSecureMarksToTaintedString(transactionId, string, mark, createNewTainted) } return string diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks-generator.js b/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks-generator.js index 5298667811e..03f37d520f4 100644 --- a/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks-generator.js +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks-generator.js @@ -3,7 +3,7 @@ let next = 0 function getNextSecureMark () { - return 1 << next++ + return (1 << next++) >>> 0 } function reset () { diff --git a/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js b/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js new file mode 100644 index 00000000000..42da281159b --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/taint-tracking/secure-marks.js @@ -0,0 +1,28 @@ +'use strict' + +const vulnerabilities = require('../vulnerabilities') +const { getNextSecureMark } = require('./secure-marks-generator') + +const marks = {} +Object.keys(vulnerabilities).forEach(vulnerability => { + marks[vulnerability + '_MARK'] = getNextSecureMark() +}) + +let asterisk = 0x0 +Object.values(marks).forEach(mark => { asterisk |= mark }) + +marks.ASTERISK_MARK = asterisk +marks.CUSTOM_SECURE_MARK = getNextSecureMark() + +function getMarkFromVulnerabilityType (vulnerabilityType) { + vulnerabilityType = vulnerabilityType?.trim() + const mark = vulnerabilityType === '*' ? 'ASTERISK_MARK' : vulnerabilityType + '_MARK' + return marks[mark] +} + +module.exports = { + ...marks, + getMarkFromVulnerabilityType, + + ALL: marks +} diff --git a/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js b/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js index 2928e566829..3d0f9013fe5 100644 --- a/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js +++ b/packages/dd-trace/src/appsec/iast/telemetry/iast-metric.js @@ -83,6 +83,9 @@ const REQUEST_TAINTED = new NoTaggedIastMetric('request.tainted', Scope.REQUEST) const EXECUTED_PROPAGATION = new NoTaggedIastMetric('executed.propagation', Scope.REQUEST) const EXECUTED_TAINTED = new NoTaggedIastMetric('executed.tainted', Scope.REQUEST) +const SUPPRESSED_VULNERABILITIES = new IastMetric('suppressed.vulnerabilities', Scope.REQUEST, + TagKey.VULNERABILITY_TYPE) + module.exports = { INSTRUMENTED_PROPAGATION, INSTRUMENTED_SOURCE, @@ -95,6 +98,8 @@ module.exports = { REQUEST_TAINTED, + SUPPRESSED_VULNERABILITIES, + PropagationType, TagKey, diff --git a/packages/dd-trace/src/appsec/iast/utils.js b/packages/dd-trace/src/appsec/iast/utils.js new file mode 100644 index 00000000000..3b09692e86d --- /dev/null +++ b/packages/dd-trace/src/appsec/iast/utils.js @@ -0,0 +1,24 @@ +'use strict' + +function iterateObjectStrings (target, fn, levelKeys = [], depth = 20, visited = new Set()) { + if (target !== null && typeof target === 'object') { + if (visited.has(target)) return + + visited.add(target) + + Object.keys(target).forEach((key) => { + const nextLevelKeys = [...levelKeys, key] + const val = target[key] + + if (typeof val === 'string') { + fn(val, nextLevelKeys, target, key) + } else if (depth > 0) { + iterateObjectStrings(val, fn, nextLevelKeys, depth - 1, visited) + } + }) + } +} + +module.exports = { + iterateObjectStrings +} diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 7e4299a0d74..ca99460fd66 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -497,6 +497,7 @@ class Config { this._setValue(defaults, 'iast.redactionNamePattern', null) this._setValue(defaults, 'iast.redactionValuePattern', null) this._setValue(defaults, 'iast.requestSampling', 30) + this._setValue(defaults, 'iast.securityControlsConfiguration', null) this._setValue(defaults, 'iast.telemetryVerbosity', 'INFORMATION') this._setValue(defaults, 'iast.stackTrace.enabled', true) this._setValue(defaults, 'injectionEnabled', []) @@ -625,6 +626,7 @@ class Config { DD_IAST_REDACTION_NAME_PATTERN, DD_IAST_REDACTION_VALUE_PATTERN, DD_IAST_REQUEST_SAMPLING, + DD_IAST_SECURITY_CONTROLS_CONFIGURATION, DD_IAST_TELEMETRY_VERBOSITY, DD_IAST_STACK_TRACE_ENABLED, DD_INJECTION_ENABLED, @@ -793,6 +795,7 @@ class Config { this._setValue(env, 'iast.requestSampling', iastRequestSampling) } this._envUnprocessed['iast.requestSampling'] = DD_IAST_REQUEST_SAMPLING + this._setString(env, 'iast.securityControlsConfiguration', DD_IAST_SECURITY_CONTROLS_CONFIGURATION) this._setString(env, 'iast.telemetryVerbosity', DD_IAST_TELEMETRY_VERBOSITY) this._setBoolean(env, 'iast.stackTrace.enabled', DD_IAST_STACK_TRACE_ENABLED) this._setArray(env, 'injectionEnabled', DD_INJECTION_ENABLED) @@ -985,8 +988,9 @@ class Config { this._setValue(opts, 'iast.requestSampling', iastRequestSampling) this._optsUnprocessed['iast.requestSampling'] = options.iast?.requestSampling } - this._setString(opts, 'iast.telemetryVerbosity', options.iast && options.iast.telemetryVerbosity) + this._setValue(opts, 'iast.securityControlsConfiguration', options.iast?.securityControlsConfiguration) this._setBoolean(opts, 'iast.stackTrace.enabled', options.iast?.stackTrace?.enabled) + this._setString(opts, 'iast.telemetryVerbosity', options.iast && options.iast.telemetryVerbosity) this._setBoolean(opts, 'isCiVisibility', options.isCiVisibility) this._setBoolean(opts, 'legacyBaggageEnabled', options.legacyBaggageEnabled) this._setBoolean(opts, 'llmobs.agentlessEnabled', options.llmobs?.agentlessEnabled) diff --git a/packages/dd-trace/src/ritm.js b/packages/dd-trace/src/ritm.js index 882e1509cdf..71bf56952cb 100644 --- a/packages/dd-trace/src/ritm.js +++ b/packages/dd-trace/src/ritm.js @@ -94,10 +94,11 @@ function Hook (modules, options, onrequire) { if (moduleLoadStartChannel.hasSubscribers) { moduleLoadStartChannel.publish(payload) } - const exports = origRequire.apply(this, arguments) + let exports = origRequire.apply(this, arguments) payload.module = exports if (moduleLoadEndChannel.hasSubscribers) { moduleLoadEndChannel.publish(payload) + exports = payload.module } // The module has already been loaded, diff --git a/packages/dd-trace/test/appsec/iast/analyzers/injection-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/injection-analyzer.spec.js new file mode 100644 index 00000000000..9673bc01808 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/analyzers/injection-analyzer.spec.js @@ -0,0 +1,100 @@ +'use strict' + +const { assert } = require('chai') +const proxyquire = require('proxyquire') +const { HTTP_REQUEST_PARAMETER, SQL_ROW_VALUE } = require('../../../../src/appsec/iast/taint-tracking/source-types') +const { SQL_INJECTION } = require('../../../../src/appsec/iast/vulnerabilities') +const { COMMAND_INJECTION_MARK, SQL_INJECTION_MARK } = + require('../../../../src/appsec/iast/taint-tracking/secure-marks') + +function getRanges (string, secureMarks, type = HTTP_REQUEST_PARAMETER) { + const range = { + start: 0, + end: string.length, + iinfo: { + parameterName: 'param', + parameterValue: string, + type + }, + secureMarks + } + + return [range] +} + +describe('InjectionAnalyzer', () => { + let analyzer, ranges + + beforeEach(() => { + ranges = [] + + const InjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/injection-analyzer', { + '../taint-tracking/operations': { + getRanges: sinon.stub().callsFake(() => ranges) + } + }) + + analyzer = new InjectionAnalyzer(SQL_INJECTION) + }) + + describe('_isVulnerable', () => { + it('should return true if no secureMarks', () => { + ranges = getRanges('tainted') + assert.isTrue(analyzer._isVulnerable('tainted')) + }) + + it('should return true if secureMarks but no SQL_INJECTION_MARK', () => { + ranges = getRanges('tainted', COMMAND_INJECTION_MARK) + assert.isTrue(analyzer._isVulnerable('tainted')) + }) + + it('should return true if some range has secureMarks but no SQL_INJECTION_MARK', () => { + ranges = [...getRanges('tainted', SQL_INJECTION), ...getRanges('tainted', COMMAND_INJECTION_MARK)] + assert.isTrue(analyzer._isVulnerable('tainted')) + }) + + it('should return false if SQL_INJECTION_MARK', () => { + ranges = getRanges('tainted', SQL_INJECTION_MARK) + assert.isFalse(analyzer._isVulnerable('tainted')) + }) + + it('should return false if combined secureMarks with SQL_INJECTION_MARK', () => { + ranges = getRanges('tainted', COMMAND_INJECTION_MARK | SQL_INJECTION_MARK) + assert.isFalse(analyzer._isVulnerable('tained')) + }) + + describe('suppressed vulnerabilities metric', () => { + const iastContext = {} + + it('should not increase metric', () => { + const incrementSuppressedMetric = sinon.stub(analyzer, '_incrementSuppressedMetric') + + ranges = getRanges('tainted', COMMAND_INJECTION_MARK) + analyzer._isVulnerable('tainted', iastContext) + + sinon.assert.notCalled(incrementSuppressedMetric) + }) + + it('should increase metric', () => { + const incrementSuppressedMetric = sinon.stub(analyzer, '_incrementSuppressedMetric') + + ranges = getRanges('tainted', SQL_INJECTION_MARK) + analyzer._isVulnerable('tainted', iastContext) + + sinon.assert.calledOnceWithExactly(incrementSuppressedMetric, iastContext) + }) + }) + + describe('with a range of SQL_ROW_VALUE input type', () => { + it('should return false if SQL_ROW_VALUE type', () => { + ranges = getRanges('tainted', undefined, SQL_ROW_VALUE) + assert.isFalse(analyzer._isVulnerable('tainted')) + }) + + it('should return true if one different from SQL_ROW_VALUE type', () => { + ranges = [...getRanges('tainted', undefined, SQL_ROW_VALUE), ...getRanges('tainted')] + assert.isTrue(analyzer._isVulnerable(ranges)) + }) + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.spec.js index 8bf10fcdf70..71e0c79d5d3 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.spec.js @@ -10,6 +10,8 @@ const { getRanges } = require('../../../../src/appsec/iast/taint-tracking/operations') +const { NOSQL_MONGODB_INJECTION_MARK } = require('../../../../src/appsec/iast/taint-tracking/secure-marks') + const sanitizeMiddlewareFinished = channel('datadog:express-mongo-sanitize:filter:finish') const sanitizeMethodFinished = channel('datadog:express-mongo-sanitize:sanitize:finish') @@ -17,7 +19,7 @@ describe('nosql injection detection in mongodb', () => { describe('SECURE_MARKS', () => { let iastContext const tid = 'transaction_id' - let nosqlInjectionMongodbAnalyzer, MONGODB_NOSQL_SECURE_MARK + let nosqlInjectionMongodbAnalyzer before(() => { nosqlInjectionMongodbAnalyzer = @@ -29,7 +31,6 @@ describe('nosql injection detection in mongodb', () => { } } }) - MONGODB_NOSQL_SECURE_MARK = nosqlInjectionMongodbAnalyzer.MONGODB_NOSQL_SECURE_MARK }) beforeEach(() => { @@ -61,7 +62,7 @@ describe('nosql injection detection in mongodb', () => { expect(sanitizedRanges.length).to.be.equal(1) expect(notSanitizedRanges.length).to.be.equal(1) - expect(sanitizedRanges[0].secureMarks).to.be.equal(MONGODB_NOSQL_SECURE_MARK) + expect(sanitizedRanges[0].secureMarks).to.be.equal(NOSQL_MONGODB_INJECTION_MARK) expect(notSanitizedRanges[0].secureMarks).to.be.equal(0) }) @@ -80,7 +81,7 @@ describe('nosql injection detection in mongodb', () => { expect(sanitizedRanges.length).to.be.equal(1) expect(notSanitizedRanges.length).to.be.equal(1) - expect(sanitizedRanges[0].secureMarks).to.be.equal(MONGODB_NOSQL_SECURE_MARK) + expect(sanitizedRanges[0].secureMarks).to.be.equal(NOSQL_MONGODB_INJECTION_MARK) expect(notSanitizedRanges[0].secureMarks).to.be.equal(0) }) }) @@ -101,7 +102,7 @@ describe('nosql injection detection in mongodb', () => { expect(notSanitizedRanges.length).to.be.equal(1) expect(notSanitizedRanges[0].secureMarks).to.be.equal(0) - expect(sanitizedRanges[0].secureMarks).to.be.equal(MONGODB_NOSQL_SECURE_MARK) + expect(sanitizedRanges[0].secureMarks).to.be.equal(NOSQL_MONGODB_INJECTION_MARK) }) it('Secure mark is added in nested objects', () => { @@ -118,7 +119,7 @@ describe('nosql injection detection in mongodb', () => { expect(sanitizedRanges.length).to.be.equal(1) expect(notSanitizedRanges.length).to.be.equal(1) - expect(sanitizedRanges[0].secureMarks).to.be.equal(MONGODB_NOSQL_SECURE_MARK) + expect(sanitizedRanges[0].secureMarks).to.be.equal(NOSQL_MONGODB_INJECTION_MARK) expect(notSanitizedRanges[0].secureMarks).to.be.equal(0) }) }) diff --git a/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js b/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js index 938c96a02c4..a37875ca1a2 100644 --- a/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js +++ b/packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js @@ -5,35 +5,57 @@ const proxyquire = require('proxyquire') const log = require('../../../../src/log') const dc = require('dc-polyfill') const { HTTP_REQUEST_PARAMETER } = require('../../../../src/appsec/iast/taint-tracking/source-types') +const { SQL_INJECTION_MARK, COMMAND_INJECTION_MARK } = + require('../../../../src/appsec/iast/taint-tracking/secure-marks') describe('sql-injection-analyzer', () => { const NOT_TAINTED_QUERY = 'no vulnerable query' const TAINTED_QUERY = 'vulnerable query' + const TAINTED_SQLI_SECURED = 'sqli secure marked vulnerable query' + const TAINTED_CMDI_SECURED = 'cmdi secure marked vulnerable query' + + function getRanges (string, secureMarks) { + const range = { + start: 0, + end: string.length, + iinfo: { + parameterName: 'param', + parameterValue: string, + type: HTTP_REQUEST_PARAMETER + }, + secureMarks + } + + return [range] + } const TaintTrackingMock = { getRanges: (iastContext, string) => { - return string === TAINTED_QUERY - ? [ - { - start: 0, - end: string.length, - iinfo: { - parameterName: 'param', - parameterValue: string, - type: HTTP_REQUEST_PARAMETER - } - } - ] - : [] + switch (string) { + case TAINTED_QUERY: + return getRanges(string) + + case TAINTED_SQLI_SECURED: + return getRanges(string, SQL_INJECTION_MARK) + + case TAINTED_CMDI_SECURED: + return getRanges(string, COMMAND_INJECTION_MARK) + + default: + return [] + } } } const InjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/injection-analyzer', { '../taint-tracking/operations': TaintTrackingMock }) - const sqlInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/sql-injection-analyzer', { + const StoredInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/stored-injection-analyzer', { './injection-analyzer': InjectionAnalyzer }) + const sqlInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/sql-injection-analyzer', { + './stored-injection-analyzer': StoredInjectionAnalyzer + }) afterEach(() => { sinon.restore() @@ -71,6 +93,16 @@ describe('sql-injection-analyzer', () => { expect(isVulnerable).to.be.true }) + it('should not detect vulnerability when vulnerable query with sqli secure mark', () => { + const isVulnerable = sqlInjectionAnalyzer._isVulnerable(TAINTED_SQLI_SECURED) + expect(isVulnerable).to.be.false + }) + + it('should detect vulnerability when vulnerable query with cmdi secure mark', () => { + const isVulnerable = sqlInjectionAnalyzer._isVulnerable(TAINTED_CMDI_SECURED) + expect(isVulnerable).to.be.true + }) + it('should report "SQL_INJECTION" vulnerability', () => { const dialect = 'DIALECT' const addVulnerability = sinon.stub() @@ -98,9 +130,14 @@ describe('sql-injection-analyzer', () => { '../taint-tracking/operations': TaintTrackingMock, './vulnerability-analyzer': ProxyAnalyzer }) + + const StoredInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/stored-injection-analyzer', { + './injection-analyzer': InjectionAnalyzer + }) + const proxiedSqlInjectionAnalyzer = proxyquire('../../../../src/appsec/iast/analyzers/sql-injection-analyzer', { - './injection-analyzer': InjectionAnalyzer, + './stored-injection-analyzer': StoredInjectionAnalyzer, '../taint-tracking/operations': TaintTrackingMock, '../iast-context': { getIastContext: () => iastContext diff --git a/packages/dd-trace/test/appsec/iast/security-controls/index.spec.js b/packages/dd-trace/test/appsec/iast/security-controls/index.spec.js new file mode 100644 index 00000000000..b393cbba375 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/index.spec.js @@ -0,0 +1,361 @@ +'use strict' + +const { assert } = require('chai') +const proxyquire = require('proxyquire') +const dc = require('dc-polyfill') +const { CUSTOM_SECURE_MARK, COMMAND_INJECTION_MARK } = + require('../../../../src/appsec/iast/taint-tracking/secure-marks') +const { saveIastContext } = require('../../../../src/appsec/iast/iast-context') + +const moduleLoadEndChannel = dc.channel('dd-trace:moduleLoadEnd') + +const CUSTOM_COMMAND_INJECTION_MARK = CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK + +describe('IAST Security Controls', () => { + let securityControls, addSecureMark, iastContext + + describe('configure', () => { + let controls, parse, startChSubscribe, endChSubscribe + + beforeEach(() => { + controls = new Map() + parse = sinon.stub().returns(controls) + startChSubscribe = sinon.stub() + endChSubscribe = sinon.stub() + + const channels = { + 'dd-trace:moduleLoadStart': { + subscribe: startChSubscribe + }, + 'dd-trace:moduleLoadEnd': { + subscribe: endChSubscribe + } + } + + securityControls = proxyquire('../../../../src/appsec/iast/security-controls', { + 'dc-polyfill': { + channel: name => channels[name] + }, + './parser': { + parse + } + }) + }) + + afterEach(() => { + securityControls.disable() + }) + + it('should call parse and subscribe to moduleLoad channels', () => { + controls.set('sanitizer.js', {}) + + const securityControlsConfiguration = 'SANITIZER:CODE_INJECTION:sanitizer.js:sanitize' + securityControls.configure({ securityControlsConfiguration }) + + sinon.assert.calledWithExactly(parse, securityControlsConfiguration) + + sinon.assert.calledOnce(startChSubscribe) + sinon.assert.calledOnce(endChSubscribe) + }) + + it('should call parse and not subscribe to moduleLoad channels', () => { + const securityControlsConfiguration = 'invalid_config' + securityControls.configure({ securityControlsConfiguration }) + + sinon.assert.calledWithExactly(parse, securityControlsConfiguration) + + sinon.assert.notCalled(startChSubscribe) + sinon.assert.notCalled(endChSubscribe) + }) + }) + + describe('hooks', () => { + beforeEach(() => { + addSecureMark = sinon.stub().callsFake((iastContext, input) => input) + + iastContext = {} + const context = {} + + securityControls = proxyquire('../../../../src/appsec/iast/security-controls', { + '../taint-tracking/operations': { + addSecureMark + }, + '../../../../../datadog-core': { + storage: () => { + return { + getStore: sinon.stub().returns(context) + } + } + } + }) + + saveIastContext(context, {}, iastContext) + }) + + afterEach(() => { + securityControls.disable() + sinon.restore() + }) + + function requireAndPublish (moduleName) { + const filename = require.resolve(moduleName) + let module = require(moduleName) + + const payload = { filename, module } + moduleLoadEndChannel.publish(payload) + module = payload.module + return module + } + + it('should hook a module only once', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate' + securityControls.configure({ securityControlsConfiguration: conf }) + + requireAndPublish('./resources/custom_input_validator') + + const { validate } = requireAndPublish('./resources/custom_input_validator') + validate('input') + + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, 'input', CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + describe('in custom libs', () => { + it('should hook configured control for input_validator', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validate } = requireAndPublish('./resources/custom_input_validator') + validate('input') + + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, 'input', CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should hook configured control for default sanitizer', () => { + // eslint-disable-next-line no-multi-str + const conf = 'SANITIZER:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/sanitizer_default.js' + securityControls.configure({ securityControlsConfiguration: conf }) + + const sanitize = requireAndPublish('./resources/sanitizer_default') + const result = sanitize('input') + + assert.equal(result, 'sanitized input') + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, true) + }) + + it('should hook multiple methods', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate;INPUT_VALIDATOR:\ +COMMAND_INJECTION:packages/dd-trace/test/appsec/iast/security-controls/resources\ +/custom_input_validator.js:validateObject' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validate, validateObject } = requireAndPublish('./resources/custom_input_validator') + let result = validate('input') + + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, false) + + result = validateObject('another input') + + sinon.assert.calledTwice(addSecureMark) + sinon.assert.calledWithExactly(addSecureMark.secondCall, + iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should hook configured control for input_validator with multiple inputs', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validate } = requireAndPublish('./resources/custom_input_validator') + validate('input1', 'input2') + + sinon.assert.calledTwice(addSecureMark) + sinon.assert.calledWithExactly(addSecureMark, iastContext, 'input1', CUSTOM_COMMAND_INJECTION_MARK, false) + sinon.assert.calledWithExactly(addSecureMark, iastContext, 'input2', CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should hook configured control for input_validator with multiple inputs marking one parameter', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate:1' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validate } = requireAndPublish('./resources/custom_input_validator') + validate('input1', 'input2') + + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, 'input2', CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should hook configured control for input_validator with multiple inputs marking multiple parameter', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate:1,3' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validate } = requireAndPublish('./resources/custom_input_validator') + validate('input1', 'input2', 'input3', 'input4') + + sinon.assert.calledTwice(addSecureMark) + sinon.assert.calledWithExactly(addSecureMark, iastContext, 'input2', CUSTOM_COMMAND_INJECTION_MARK, false) + sinon.assert.calledWithExactly(addSecureMark, iastContext, 'input4', CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should hook configured control for input_validator with invalid parameter', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validate:42' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validate } = requireAndPublish('./resources/custom_input_validator') + validate('input1') + + sinon.assert.notCalled(addSecureMark) + }) + + it('should hook configured control for sanitizer', () => { + // eslint-disable-next-line no-multi-str + const conf = 'SANITIZER:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/sanitizer.js:sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { sanitize } = requireAndPublish('./resources/sanitizer') + const result = sanitize('input') + + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, true) + }) + }) + + describe('object inputs or sanitized outputs', () => { + it('should add marks for input string properties', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validateObject' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validateObject } = requireAndPublish('./resources/custom_input_validator') + const result = validateObject({ input1: 'input1', nested: { input: 'input2' } }) + + sinon.assert.calledTwice(addSecureMark) + sinon.assert.calledWithExactly(addSecureMark.firstCall, + iastContext, result.input1, CUSTOM_COMMAND_INJECTION_MARK, false) + sinon.assert.calledWithExactly(addSecureMark.secondCall, + iastContext, result.nested.input, CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should add marks for mixed input string properties', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/custom_input_validator.js:validateObject' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { validateObject } = requireAndPublish('./resources/custom_input_validator') + const result = validateObject({ input1: 'input1' }, 'input3') + + sinon.assert.calledTwice(addSecureMark) + sinon.assert.calledWithExactly(addSecureMark.firstCall, + iastContext, result.input1, CUSTOM_COMMAND_INJECTION_MARK, false) + sinon.assert.calledWithExactly(addSecureMark.secondCall, + iastContext, 'input3', CUSTOM_COMMAND_INJECTION_MARK, false) + }) + + it('should add marks for sanitized object string properties', () => { + // eslint-disable-next-line no-multi-str + const conf = 'SANITIZER:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/sanitizer.js:sanitizeObject' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { sanitizeObject } = requireAndPublish('./resources/sanitizer') + const result = sanitizeObject({ output: 'output1', nested: { output: 'nested output' } }) + + sinon.assert.calledTwice(addSecureMark) + sinon.assert.calledWithExactly(addSecureMark.firstCall, + iastContext, result.output, CUSTOM_COMMAND_INJECTION_MARK, true) + sinon.assert.calledWithExactly(addSecureMark.secondCall, + iastContext, result.nested.output, CUSTOM_COMMAND_INJECTION_MARK, true) + }) + }) + + describe('in nested objects', () => { + it('should hook configured control for sanitizer in nested object', () => { + // eslint-disable-next-line no-multi-str + const conf = 'SANITIZER:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/sanitizer.js:nested.sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { nested } = requireAndPublish('./resources/sanitizer') + const result = nested.sanitize('input') + + assert.equal(result, 'sanitized input') + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, true) + }) + + it('should not fail hook in incorrect nested object', () => { + // eslint-disable-next-line no-multi-str + const conf = 'SANITIZER:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/sanitizer.js:incorrect.sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { nested } = requireAndPublish('./resources/sanitizer') + const result = nested.sanitize('input') + + sinon.assert.notCalled(addSecureMark) + assert.equal(result, 'sanitized input') + }) + + it('should not fail hook in incorrect nested object 2', () => { + // eslint-disable-next-line no-multi-str + const conf = 'SANITIZER:COMMAND_INJECTION:packages/dd-trace/test/appsec/iast\ +/security-controls/resources/sanitizer.js:nested.incorrect.sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { nested } = requireAndPublish('./resources/sanitizer') + const result = nested.sanitize('input') + + sinon.assert.notCalled(addSecureMark) + assert.equal(result, 'sanitized input') + }) + }) + + describe('in node_modules', () => { + it('should hook node_module dependency', () => { + const conf = 'SANITIZER:COMMAND_INJECTION:node_modules/sanitizer/index.js:sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { sanitize } = requireAndPublish('./resources/node_modules/sanitizer') + const result = sanitize('input') + + assert.equal(result, 'sanitized input') + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, true) + }) + + it('should hook transitive node_module dependency', () => { + const conf = 'SANITIZER:COMMAND_INJECTION:node_modules/sanitizer/index.js:sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { sanitize } = requireAndPublish('./resources/node_modules/anotherlib/node_modules/sanitizer') + const result = sanitize('input') + + assert.equal(result, 'sanitized input') + sinon.assert.calledOnceWithExactly(addSecureMark, iastContext, result, CUSTOM_COMMAND_INJECTION_MARK, true) + }) + + it('should not fail with not found node_module dep', () => { + const conf = 'SANITIZER:COMMAND_INJECTION:node_modules/not_loaded_sanitizer/index.js:sanitize' + securityControls.configure({ securityControlsConfiguration: conf }) + + const { sanitize } = requireAndPublish('./resources/node_modules/sanitizer') + const result = sanitize('input') + + assert.equal(result, 'sanitized input') + sinon.assert.notCalled(addSecureMark) + }) + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/security-controls/parser.spec.js b/packages/dd-trace/test/appsec/iast/security-controls/parser.spec.js new file mode 100644 index 00000000000..179ea130767 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/parser.spec.js @@ -0,0 +1,288 @@ +'use strict' + +const { assert } = require('chai') +const { parse } = require('../../../../src/appsec/iast/security-controls/parser') + +const { + COMMAND_INJECTION_MARK, + CODE_INJECTION_MARK, + CUSTOM_SECURE_MARK, + ASTERISK_MARK +} = require('../../../../src/appsec/iast/taint-tracking/secure-marks') + +const civFilename = 'bar/foo/custom_input_validator.js' +const sanitizerFilename = 'bar/foo/sanitizer.js' + +describe('IAST Security Controls parser', () => { + describe('parse', () => { + it('should not parse invalid type', () => { + const conf = 'INVALID_TYPE:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.isUndefined(civ) + }) + + it('should not parse invalid security control definition with extra fields', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate:1:extra_invalid' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.isUndefined(civ) + }) + + it('should not parse invalid security mark security control definition', () => { + const conf = 'INPUT_VALIDATOR:INVALID_MARK:bar/foo/custom_input_validator.js:validate:1' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.isUndefined(civ) + }) + + it('should not parse invalid parameter in security control definition', () => { + const conf = 'INPUT_VALIDATOR:INVALID_MARK:bar/foo/custom_input_validator.js:validate:not_numeric_parameter' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.isUndefined(civ) + }) + + it('should parse valid simple security control definition without parameters', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + expect(civ).not.undefined + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: undefined + }) + }) + + it('should parse valid simple security control definition for a sanitizer', () => { + const conf = 'SANITIZER:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'SANITIZER', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: undefined + }) + }) + + it('should parse valid simple security control definition for a sanitizer without method', () => { + const conf = 'SANITIZER:COMMAND_INJECTION:bar/foo/custom_input_validator.js' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'SANITIZER', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: undefined, + parameters: undefined + }) + }) + + it('should parse security control definition containing spaces or alike', () => { + const conf = `INPUT_VALIDATOR : COMMAND_INJECTION: + bar/foo/custom_input_validator.js: validate` + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: undefined + }) + }) + + it('should parse valid simple security control definition with multiple marks', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION, CODE_INJECTION:bar/foo/custom_input_validator.js:validate' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK | CODE_INJECTION_MARK, + method: 'validate', + parameters: undefined + }) + }) + + it('should parse valid simple security control definition with multiple marks ignoring empty values', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION, CODE_INJECTION, , :bar/foo/custom_input_validator.js:validate' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK | CODE_INJECTION_MARK, + method: 'validate', + parameters: undefined + }) + }) + + it('should parse valid simple security control definition within exported object', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validator.validate' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validator.validate', + parameters: undefined + }) + }) + + it('should parse valid simple security control definition within exported object and parameter', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validator.validate:1' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validator.validate', + parameters: [1] + }) + }) + + it('should parse valid simple security control definition with one parameter', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate:0' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: [0] + }) + }) + + it('should parse valid simple security control definition with multiple parameters', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate:1,2' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename)[0] + + assert.deepStrictEqual(civ, { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: [1, 2] + }) + }) + + it('should parse valid multiple security control definitions for the same file', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate:1,2;\ +SANITIZER:COMMAND_INJECTION:bar/foo/custom_input_validator.js:sanitize' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.deepStrictEqual(civ[0], { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: [1, 2] + }) + + assert.deepStrictEqual(civ[1], { + type: 'SANITIZER', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'sanitize', + parameters: undefined + }) + }) + + it('should parse valid multiple security control definitions for different files', () => { + // eslint-disable-next-line no-multi-str + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate:1,2;\ +SANITIZER:COMMAND_INJECTION:bar/foo/sanitizer.js:sanitize' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.deepStrictEqual(civ[0], { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: [1, 2] + }) + + const sanitizerJs = securityControls.get(sanitizerFilename) + assert.deepStrictEqual(sanitizerJs[0], { + type: 'SANITIZER', + file: sanitizerFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'sanitize', + parameters: undefined + }) + }) + + it('should parse valid multiple security control definitions for different files ignoring empty', () => { + const conf = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar/foo/custom_input_validator.js:validate:1,2;;' + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.deepStrictEqual(civ[0], { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | COMMAND_INJECTION_MARK, + method: 'validate', + parameters: [1, 2] + }) + }) + + it('should parse * marks', () => { + const conf = 'INPUT_VALIDATOR:*:bar/foo/custom_input_validator.js:validate:1,2' + + const securityControls = parse(conf) + + const civ = securityControls.get(civFilename) + + assert.deepStrictEqual(civ[0], { + type: 'INPUT_VALIDATOR', + file: civFilename, + secureMarks: CUSTOM_SECURE_MARK | ASTERISK_MARK, + method: 'validate', + parameters: [1, 2] + }) + }) + }) +}) diff --git a/packages/dd-trace/test/appsec/iast/security-controls/resources/custom_input_validator.js b/packages/dd-trace/test/appsec/iast/security-controls/resources/custom_input_validator.js new file mode 100644 index 00000000000..164d0dc3d92 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/resources/custom_input_validator.js @@ -0,0 +1,14 @@ +'use strict' + +function validate (input) { + return input +} + +function validateObject (input) { + return input +} + +module.exports = { + validate, + validateObject +} diff --git a/packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/anotherlib/node_modules/sanitizer/index.js b/packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/anotherlib/node_modules/sanitizer/index.js new file mode 100644 index 00000000000..66009794124 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/anotherlib/node_modules/sanitizer/index.js @@ -0,0 +1,13 @@ +'use strict' + +function sanitize (input) { + return `sanitized ${input}` +} + +module.exports = { + sanitize, + + nested: { + sanitize + } +} diff --git a/packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/sanitizer/index.js b/packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/sanitizer/index.js new file mode 100644 index 00000000000..66009794124 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/resources/node_modules/sanitizer/index.js @@ -0,0 +1,13 @@ +'use strict' + +function sanitize (input) { + return `sanitized ${input}` +} + +module.exports = { + sanitize, + + nested: { + sanitize + } +} diff --git a/packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer.js b/packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer.js new file mode 100644 index 00000000000..1304bceb621 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer.js @@ -0,0 +1,18 @@ +'use strict' + +function sanitize (input) { + return `sanitized ${input}` +} + +function sanitizeObject (input) { + return { sanitized: true, ...input } +} + +module.exports = { + sanitize, + sanitizeObject, + + nested: { + sanitize + } +} diff --git a/packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer_default.js b/packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer_default.js new file mode 100644 index 00000000000..cfb9b08a90f --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/security-controls/resources/sanitizer_default.js @@ -0,0 +1,7 @@ +'use strict' + +function sanitize (input) { + return `sanitized ${input}` +} + +module.exports = sanitize diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks-generator.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks-generator.spec.js index e5ddb8b6bbe..75e54825fb0 100644 --- a/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks-generator.spec.js +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks-generator.spec.js @@ -12,7 +12,7 @@ describe('test secure marks generator', () => { it('should generate numbers in order', () => { for (let i = 0; i < 100; i++) { - expect(getNextSecureMark()).to.be.equal(1 << i) + expect(getNextSecureMark()).to.be.equal((1 << i) >>> 0) } }) }) diff --git a/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks.spec.js b/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks.spec.js new file mode 100644 index 00000000000..b6f61f8bed0 --- /dev/null +++ b/packages/dd-trace/test/appsec/iast/taint-tracking/secure-marks.spec.js @@ -0,0 +1,34 @@ +'use strict' + +const { assert } = require('chai') +const { + SQL_INJECTION_MARK, + getMarkFromVulnerabilityType, + ASTERISK_MARK, + ALL +} = require('../../../../src/appsec/iast/taint-tracking/secure-marks') +const { SQL_INJECTION } = require('../../../../src/appsec/iast/vulnerabilities') + +describe('IAST secure marks', () => { + it('should generate a mark for each vulnerability', () => { + const mark = getMarkFromVulnerabilityType(SQL_INJECTION) + assert.equal(mark, SQL_INJECTION_MARK) + }) + + it('should generate a mark for every vulnerability', () => { + const mark = getMarkFromVulnerabilityType('*') + assert.equal(mark, ASTERISK_MARK) + }) + + it('should not be repeated marks (probably due to truncation)', () => { + const markValues = Object.values(ALL) + assert.equal(markValues.length, [...new Set(markValues)].length) + }) + + it('should generate marks under 0x100000000 due taint-tracking secure mark length', () => { + // in theory secure-marks generator can not reach this value with bitwise operations due to 32-bit integer linmits + const limitMark = 0x100000000 + + Object.values(ALL).forEach(mark => assert.isTrue(mark < limitMark)) + }) +}) diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index 56cda03cf4e..0ae94e5fca0 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -331,6 +331,7 @@ describe('Config', () => { { name: 'iast.redactionNamePattern', value: null, origin: 'default' }, { name: 'iast.redactionValuePattern', value: null, origin: 'default' }, { name: 'iast.requestSampling', value: 30, origin: 'default' }, + { name: 'iast.securityControlsConfiguration', value: null, origin: 'default' }, { name: 'iast.telemetryVerbosity', value: 'INFORMATION', origin: 'default' }, { name: 'iast.stackTrace.enabled', value: true, origin: 'default' }, { name: 'injectionEnabled', value: [], origin: 'default' }, @@ -505,6 +506,7 @@ describe('Config', () => { process.env.DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS = '42' process.env.DD_IAST_ENABLED = 'true' process.env.DD_IAST_REQUEST_SAMPLING = '40' + process.env.DD_IAST_SECURITY_CONTROLS_CONFIGURATION = 'SANITIZER:CODE_INJECTION:sanitizer.js:method' process.env.DD_IAST_MAX_CONCURRENT_REQUESTS = '3' process.env.DD_IAST_MAX_CONTEXT_OPERATIONS = '4' process.env.DD_IAST_COOKIE_FILTER_PATTERN = '.*' @@ -629,6 +631,8 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionEnabled', false) expect(config).to.have.nested.property('iast.redactionNamePattern', 'REDACTION_NAME_PATTERN') expect(config).to.have.nested.property('iast.redactionValuePattern', 'REDACTION_VALUE_PATTERN') + expect(config).to.have.nested.property('iast.securityControlsConfiguration', + 'SANITIZER:CODE_INJECTION:sanitizer.js:method') expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') expect(config).to.have.nested.property('iast.stackTrace.enabled', false) expect(config).to.have.deep.property('installSignature', { @@ -681,6 +685,11 @@ describe('Config', () => { { name: 'iast.redactionNamePattern', value: 'REDACTION_NAME_PATTERN', origin: 'env_var' }, { name: 'iast.redactionValuePattern', value: 'REDACTION_VALUE_PATTERN', origin: 'env_var' }, { name: 'iast.requestSampling', value: '40', origin: 'env_var' }, + { + name: 'iast.securityControlsConfiguration', + value: 'SANITIZER:CODE_INJECTION:sanitizer.js:method', + origin: 'env_var' + }, { name: 'iast.telemetryVerbosity', value: 'DEBUG', origin: 'env_var' }, { name: 'iast.stackTrace.enabled', value: false, origin: 'env_var' }, { name: 'instrumentation_config_id', value: 'abcdef123', origin: 'env_var' }, @@ -883,6 +892,7 @@ describe('Config', () => { redactionEnabled: false, redactionNamePattern: 'REDACTION_NAME_PATTERN', redactionValuePattern: 'REDACTION_VALUE_PATTERN', + securityControlsConfiguration: 'SANITIZER:CODE_INJECTION:sanitizer.js:method', telemetryVerbosity: 'DEBUG', stackTrace: { enabled: false @@ -962,8 +972,10 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionEnabled', false) expect(config).to.have.nested.property('iast.redactionNamePattern', 'REDACTION_NAME_PATTERN') expect(config).to.have.nested.property('iast.redactionValuePattern', 'REDACTION_VALUE_PATTERN') - expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') + expect(config).to.have.nested.property('iast.securityControlsConfiguration', + 'SANITIZER:CODE_INJECTION:sanitizer.js:method') expect(config).to.have.nested.property('iast.stackTrace.enabled', false) + expect(config).to.have.nested.property('iast.telemetryVerbosity', 'DEBUG') expect(config).to.have.deep.nested.property('sampler', { sampleRate: 0.5, rateLimit: 1000, @@ -1017,6 +1029,11 @@ describe('Config', () => { { name: 'iast.redactionNamePattern', value: 'REDACTION_NAME_PATTERN', origin: 'code' }, { name: 'iast.redactionValuePattern', value: 'REDACTION_VALUE_PATTERN', origin: 'code' }, { name: 'iast.requestSampling', value: 50, origin: 'code' }, + { + name: 'iast.securityControlsConfiguration', + value: 'SANITIZER:CODE_INJECTION:sanitizer.js:method', + origin: 'code' + }, { name: 'iast.telemetryVerbosity', value: 'DEBUG', origin: 'code' }, { name: 'iast.stackTrace.enabled', value: false, origin: 'code' }, { name: 'middlewareTracingEnabled', value: false, origin: 'code' }, @@ -1244,6 +1261,7 @@ describe('Config', () => { process.env.DD_IAST_REDACTION_NAME_PATTERN = 'name_pattern_to_be_overriden_by_options' process.env.DD_IAST_REDACTION_VALUE_PATTERN = 'value_pattern_to_be_overriden_by_options' process.env.DD_IAST_STACK_TRACE_ENABLED = 'true' + process.env.DD_IAST_SECURITY_CONTROLS_CONFIGURATION = 'SANITIZER:CODE_INJECTION:sanitizer.js:method1' process.env.DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED = 'true' process.env.DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = 'true' process.env.DD_LLMOBS_ML_APP = 'myMlApp' @@ -1326,6 +1344,7 @@ describe('Config', () => { dbRowsToTaint: 3, redactionNamePattern: 'REDACTION_NAME_PATTERN', redactionValuePattern: 'REDACTION_VALUE_PATTERN', + securityControlsConfiguration: 'SANITIZER:CODE_INJECTION:sanitizer.js:method2', stackTrace: { enabled: false } @@ -1405,6 +1424,8 @@ describe('Config', () => { expect(config).to.have.nested.property('iast.redactionNamePattern', 'REDACTION_NAME_PATTERN') expect(config).to.have.nested.property('iast.redactionValuePattern', 'REDACTION_VALUE_PATTERN') expect(config).to.have.nested.property('iast.stackTrace.enabled', false) + expect(config).to.have.nested.property('iast.securityControlsConfiguration', + 'SANITIZER:CODE_INJECTION:sanitizer.js:method2') expect(config).to.have.nested.property('llmobs.mlApp', 'myOtherMlApp') expect(config).to.have.nested.property('llmobs.agentlessEnabled', false) }) @@ -1531,6 +1552,7 @@ describe('Config', () => { redactionEnabled: false, redactionNamePattern: 'REDACTION_NAME_PATTERN', redactionValuePattern: 'REDACTION_VALUE_PATTERN', + securityControlsConfiguration: null, telemetryVerbosity: 'DEBUG', stackTrace: { enabled: false diff --git a/packages/dd-trace/test/ritm-tests/module-default.js b/packages/dd-trace/test/ritm-tests/module-default.js new file mode 100644 index 00000000000..46733ba5804 --- /dev/null +++ b/packages/dd-trace/test/ritm-tests/module-default.js @@ -0,0 +1,5 @@ +'use strict' + +module.exports = function () { + return 'hi' +} diff --git a/packages/dd-trace/test/ritm.spec.js b/packages/dd-trace/test/ritm.spec.js index df2a4e8b1a4..6d7a5517143 100644 --- a/packages/dd-trace/test/ritm.spec.js +++ b/packages/dd-trace/test/ritm.spec.js @@ -65,6 +65,24 @@ describe('Ritm', () => { assert.equal(a(), 'Called by AJ') }) + it('should allow override original module', () => { + const onModuleLoadEnd = (payload) => { + if (payload.request === './ritm-tests/module-default') { + payload.module = function () { + return 'ho' + } + } + } + + moduleLoadEndChannel.subscribe(onModuleLoadEnd) + try { + const hi = require('./ritm-tests/module-default') + assert.equal(hi(), 'ho') + } finally { + moduleLoadEndChannel.unsubscribe(onModuleLoadEnd) + } + }) + it('should fall back to monkey patched module', () => { assert.equal(require('http').foo, 1, 'normal hooking still works') diff --git a/yarn.lock b/yarn.lock index 49b77442fb6..2c240734dd8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -421,10 +421,10 @@ lru-cache "^7.14.0" node-gyp-build "^4.5.0" -"@datadog/native-iast-taint-tracking@3.2.0": - version "3.2.0" - resolved "https://registry.yarnpkg.com/@datadog/native-iast-taint-tracking/-/native-iast-taint-tracking-3.2.0.tgz#9fb6823d82f934e12c06ea1baa7399ca80deb2ec" - integrity sha512-Mc6FzCoyvU5yXLMsMS9yKnEqJMWoImAukJXolNWCTm+JQYCMf2yMsJ8pBAm7KyZKliamM9rCn7h7Tr2H3lXwjA== +"@datadog/native-iast-taint-tracking@3.3.0": + version "3.3.0" + resolved "https://registry.yarnpkg.com/@datadog/native-iast-taint-tracking/-/native-iast-taint-tracking-3.3.0.tgz#5a9c87e07376e7c5a4b4d4985f140a60388eee00" + integrity sha512-OzmjOncer199ATSYeCAwSACCRyQimo77LKadSHDUcxa/n9FYU+2U/bYQTYsK3vquSA2E47EbSVq9rytrlTdvnA== dependencies: node-gyp-build "^3.9.0"