Skip to content

Commit

Permalink
Template injection vulnerability detection in handlebars and pug (#4827)
Browse files Browse the repository at this point in the history
* Template injection vulnerability detection in handlebars

* template injection vulnerability detection in pug

* fix lint and naming issues

* create separate job for template injection

* add support to registerPartial function

* add tests for pug render function
  • Loading branch information
IlyasShabi authored Nov 15, 2024
1 parent 59e9a2a commit 985cb1d
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 10 deletions.
14 changes: 14 additions & 0 deletions .github/workflows/appsec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,17 @@ jobs:
- uses: ./.github/actions/node/latest
- run: yarn test:appsec:plugins:ci
- uses: codecov/codecov-action@v3

template:
runs-on: ubuntu-latest
env:
PLUGINS: handlebars|pug
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/node/setup
- uses: ./.github/actions/install
- uses: ./.github/actions/node/oldest
- run: yarn test:appsec:plugins:ci
- uses: ./.github/actions/node/latest
- run: yarn test:appsec:plugins:ci
- uses: codecov/codecov-action@v3
40 changes: 40 additions & 0 deletions packages/datadog-instrumentations/src/handlebars.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict'

const shimmer = require('../../datadog-shimmer')
const { channel, addHook } = require('./helpers/instrument')

const handlebarsCompileCh = channel('datadog:handlebars:compile:start')
const handlebarsRegisterPartialCh = channel('datadog:handlebars:register-partial:start')

function wrapCompile (compile) {
return function wrappedCompile (source) {
if (handlebarsCompileCh.hasSubscribers) {
handlebarsCompileCh.publish({ source })
}

return compile.apply(this, arguments)
}
}

function wrapRegisterPartial (registerPartial) {
return function wrappedRegisterPartial (name, partial) {
if (handlebarsRegisterPartialCh.hasSubscribers) {
handlebarsRegisterPartialCh.publish({ partial })
}

return registerPartial.apply(this, arguments)
}
}

addHook({ name: 'handlebars', file: 'dist/cjs/handlebars/compiler/compiler.js', versions: ['>=4.0.0'] }, compiler => {
shimmer.wrap(compiler, 'compile', wrapCompile)
shimmer.wrap(compiler, 'precompile', wrapCompile)

return compiler
})

addHook({ name: 'handlebars', file: 'dist/cjs/handlebars/base.js', versions: ['>=4.0.0'] }, base => {
shimmer.wrap(base.HandlebarsEnvironment.prototype, 'registerPartial', wrapRegisterPartial)

return base
})
2 changes: 2 additions & 0 deletions packages/datadog-instrumentations/src/helpers/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ module.exports = {
'generic-pool': () => require('../generic-pool'),
graphql: () => require('../graphql'),
grpc: () => require('../grpc'),
handlebars: () => require('../handlebars'),
hapi: () => require('../hapi'),
http: () => require('../http'),
http2: () => require('../http2'),
Expand Down Expand Up @@ -105,6 +106,7 @@ module.exports = {
'promise-js': () => require('../promise-js'),
promise: () => require('../promise'),
protobufjs: () => require('../protobufjs'),
pug: () => require('../pug'),
q: () => require('../q'),
qs: () => require('../qs'),
redis: () => require('../redis'),
Expand Down
23 changes: 23 additions & 0 deletions packages/datadog-instrumentations/src/pug.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict'

const shimmer = require('../../datadog-shimmer')
const { channel, addHook } = require('./helpers/instrument')

const pugCompileCh = channel('datadog:pug:compile:start')

function wrapCompile (compile) {
return function wrappedCompile (source) {
if (pugCompileCh.hasSubscribers) {
pugCompileCh.publish({ source })
}

return compile.apply(this, arguments)
}
}

addHook({ name: 'pug', versions: ['>=2.0.4'] }, compiler => {
shimmer.wrap(compiler, 'compile', wrapCompile)
shimmer.wrap(compiler, 'compileClientWithDependenciesTracked', wrapCompile)

return compiler
})
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/iast/analyzers/analyzers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module.exports = {
PATH_TRAVERSAL_ANALYZER: require('./path-traversal-analyzer'),
SQL_INJECTION_ANALYZER: require('./sql-injection-analyzer'),
SSRF: require('./ssrf-analyzer'),
TEMPLATE_INJECTION_ANALYZER: require('./template-injection-analyzer'),
UNVALIDATED_REDIRECT_ANALYZER: require('./unvalidated-redirect-analyzer'),
WEAK_CIPHER_ANALYZER: require('./weak-cipher-analyzer'),
WEAK_HASH_ANALYZER: require('./weak-hash-analyzer'),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict'

const InjectionAnalyzer = require('./injection-analyzer')
const { TEMPLATE_INJECTION } = require('../vulnerabilities')

class TemplateInjectionAnalyzer extends InjectionAnalyzer {
constructor () {
super(TEMPLATE_INJECTION)
}

onConfigure () {
this.addSub('datadog:handlebars:compile:start', ({ source }) => this.analyze(source))
this.addSub('datadog:handlebars:register-partial:start', ({ partial }) => this.analyze(partial))
this.addSub('datadog:pug:compile:start', ({ source }) => this.analyze(source))
}
}

module.exports = new TemplateInjectionAnalyzer()
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ const vulnerabilities = require('../../vulnerabilities')

const { contains, intersects, remove } = require('./range-utils')

const codeInjectionSensitiveAnalyzer = require('./sensitive-analyzers/code-injection-sensitive-analyzer')
const commandSensitiveAnalyzer = require('./sensitive-analyzers/command-sensitive-analyzer')
const hardcodedPasswordAnalyzer = require('./sensitive-analyzers/hardcoded-password-analyzer')
const headerSensitiveAnalyzer = require('./sensitive-analyzers/header-sensitive-analyzer')
const jsonSensitiveAnalyzer = require('./sensitive-analyzers/json-sensitive-analyzer')
const ldapSensitiveAnalyzer = require('./sensitive-analyzers/ldap-sensitive-analyzer')
const sqlSensitiveAnalyzer = require('./sensitive-analyzers/sql-sensitive-analyzer')
const taintedRangeBasedSensitiveAnalyzer = require('./sensitive-analyzers/tainted-range-based-sensitive-analyzer')
const urlSensitiveAnalyzer = require('./sensitive-analyzers/url-sensitive-analyzer')

const { DEFAULT_IAST_REDACTION_NAME_PATTERN, DEFAULT_IAST_REDACTION_VALUE_PATTERN } = require('./sensitive-regex')
Expand All @@ -24,7 +24,8 @@ class SensitiveHandler {
this._valuePattern = new RegExp(DEFAULT_IAST_REDACTION_VALUE_PATTERN, 'gmi')

this._sensitiveAnalyzers = new Map()
this._sensitiveAnalyzers.set(vulnerabilities.CODE_INJECTION, codeInjectionSensitiveAnalyzer)
this._sensitiveAnalyzers.set(vulnerabilities.CODE_INJECTION, taintedRangeBasedSensitiveAnalyzer)
this._sensitiveAnalyzers.set(vulnerabilities.TEMPLATE_INJECTION, taintedRangeBasedSensitiveAnalyzer)
this._sensitiveAnalyzers.set(vulnerabilities.COMMAND_INJECTION, commandSensitiveAnalyzer)
this._sensitiveAnalyzers.set(vulnerabilities.NOSQL_MONGODB_INJECTION, jsonSensitiveAnalyzer)
this._sensitiveAnalyzers.set(vulnerabilities.LDAP_INJECTION, ldapSensitiveAnalyzer)
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/iast/vulnerabilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
PATH_TRAVERSAL: 'PATH_TRAVERSAL',
SQL_INJECTION: 'SQL_INJECTION',
SSRF: 'SSRF',
TEMPLATE_INJECTION: 'TEMPLATE_INJECTION',
UNVALIDATED_REDIRECT: 'UNVALIDATED_REDIRECT',
WEAK_CIPHER: 'WEAK_CIPHER',
WEAK_HASH: 'WEAK_HASH',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
'use strict'

const { prepareTestServerForIast } = require('../utils')
const { storage } = require('../../../../../datadog-core')
const iastContextFunctions = require('../../../../src/appsec/iast/iast-context')
const { newTaintedString } = require('../../../../src/appsec/iast/taint-tracking/operations')

describe('template-injection-analyzer with handlebars', () => {
withVersions('handlebars', 'handlebars', version => {
let source
before(() => {
source = '<p>{{name}}</p>'
})

describe('compile', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/handlebars@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const template = newTaintedString(iastContext, source, 'param', 'Request')
lib.compile(template)
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
lib.compile(source)
}, 'TEMPLATE_INJECTION')
})
})

describe('precompile', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/handlebars@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const template = newTaintedString(iastContext, source, 'param', 'Request')
lib.precompile(template)
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
lib.precompile(source)
}, 'TEMPLATE_INJECTION')
})
})

describe('registerPartial', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/handlebars@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const partial = newTaintedString(iastContext, source, 'param', 'Request')

lib.registerPartial('vulnerablePartial', partial)
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
lib.registerPartial('vulnerablePartial', source)
}, 'TEMPLATE_INJECTION')
})
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
'use strict'

const { prepareTestServerForIast } = require('../utils')
const { storage } = require('../../../../../datadog-core')
const iastContextFunctions = require('../../../../src/appsec/iast/iast-context')
const { newTaintedString } = require('../../../../src/appsec/iast/taint-tracking/operations')

describe('template-injection-analyzer with pug', () => {
withVersions('pug', 'pug', version => {
let source
before(() => {
source = 'string of pug'
})

describe('compile', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/pug@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const template = newTaintedString(iastContext, source, 'param', 'Request')
lib.compile(template)
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
const template = lib.compile(source)
template()
}, 'TEMPLATE_INJECTION')
})
})

describe('compileClient', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/pug@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const template = newTaintedString(iastContext, source, 'param', 'Request')
lib.compileClient(template)
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
lib.compileClient(source)
}, 'TEMPLATE_INJECTION')
})
})

describe('compileClientWithDependenciesTracked', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/pug@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const template = newTaintedString(iastContext, source, 'param', 'Request')
lib.compileClientWithDependenciesTracked(template, {})
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
lib.compileClient(source)
}, 'TEMPLATE_INJECTION')
})
})

describe('render', () => {
prepareTestServerForIast('template injection analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
let lib
beforeEach(() => {
lib = require(`../../../../../../versions/pug@${version}`).get()
})

testThatRequestHasVulnerability(() => {
const store = storage.getStore()
const iastContext = iastContextFunctions.getIastContext(store)
const str = newTaintedString(iastContext, source, 'param', 'Request')
lib.render(str)
}, 'TEMPLATE_INJECTION')

testThatRequestHasNoVulnerability(() => {
lib.render(source)
}, 'TEMPLATE_INJECTION')
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const excludedVulnerabilityTypes = ['XSS', 'EMAIL_HTML_INJECTION']
const excludedTests = [
'Query with single quoted string literal and null source', // does not apply
'Redacted source that needs to be truncated', // not implemented yet
'CODE_INJECTION - Tainted range based redaction - with null source ' // does not apply
'CODE_INJECTION - Tainted range based redaction - with null source ', // does not apply
'TEMPLATE_INJECTION - Tainted range based redaction - with null source ' // does not apply
]

function doTest (testCase, parameters) {
Expand Down
Loading

0 comments on commit 985cb1d

Please sign in to comment.