Skip to content

Commit a2acbc2

Browse files
committed
Fix LDAPi vulnerability location
1 parent 5491b44 commit a2acbc2

File tree

5 files changed

+83
-2
lines changed

5 files changed

+83
-2
lines changed

packages/dd-trace/src/appsec/iast/analyzers/ldap-injection-analyzer.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict'
22
const InjectionAnalyzer = require('./injection-analyzer')
33
const { LDAP_INJECTION } = require('../vulnerabilities')
4+
const { getNodeModulesPaths } = require('../path-line')
5+
6+
const EXCLUDED_PATHS = getNodeModulesPaths('ldapjs-promise')
47

58
class LdapInjectionAnalyzer extends InjectionAnalyzer {
69
constructor () {
@@ -10,6 +13,10 @@ class LdapInjectionAnalyzer extends InjectionAnalyzer {
1013
onConfigure () {
1114
this.addSub('datadog:ldapjs:client:search', ({ base, filter }) => this.analyzeAll(base, filter))
1215
}
16+
17+
_getExcludedPaths () {
18+
return EXCLUDED_PATHS
19+
}
1320
}
1421

1522
module.exports = new LdapInjectionAnalyzer()

packages/dd-trace/src/appsec/iast/path-line.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ const pathLine = {
88
getNodeModulesPaths,
99
getFirstNonDDPathAndLineFromCallsites, // Exported only for test purposes
1010
calculateDDBasePath, // Exported only for test purposes
11-
ddBasePath: calculateDDBasePath(__dirname) // Only for test purposes
11+
ddBasePath: calculateDDBasePath(__dirname), // Only for test purposes
12+
ddBasePathVersions: path.join(calculateDDBasePath(__dirname), 'versions') // Only for test purposes
1213
}
1314

1415
const EXCLUDED_PATHS = [
@@ -43,7 +44,10 @@ function getFirstNonDDPathAndLineFromCallsites (callsites, externallyExcludedPat
4344
for (let i = 0; i < callsites.length; i++) {
4445
const callsite = callsites[i]
4546
const filepath = callsite.getFileName()
46-
if (!isExcluded(callsite, externallyExcludedPaths) && filepath.indexOf(pathLine.ddBasePath) === -1) {
47+
if (
48+
!isExcluded(callsite, externallyExcludedPaths) &&
49+
(filepath.indexOf(pathLine.ddBasePathVersions) > -1 || filepath.indexOf(pathLine.ddBasePath) === -1)
50+
) {
4751
return {
4852
path: path.relative(process.cwd(), filepath),
4953
line: callsite.getLineNumber(),

packages/dd-trace/test/appsec/iast/analyzers/ldap-injection-analyzer.ldapjs.plugin.spec.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
'use strict'
22

3+
const fs = require('fs')
4+
const os = require('os')
5+
const path = require('path')
6+
37
const { prepareTestServerForIast } = require('../utils')
48
const { storage } = require('../../../../../datadog-core')
59
const iastContextFunctions = require('../../../../src/appsec/iast/iast-context')
@@ -165,4 +169,53 @@ describe('ldap-injection-analyzer with ldapjs', () => {
165169
})
166170
})
167171
})
172+
173+
withVersions('ldapjs', 'ldapjs-promise', promiseVersion => {
174+
prepareTestServerForIast('ldapjs-promise', (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
175+
const srcFilePath = path.join(__dirname, 'resources', 'ldap-injection-methods.js')
176+
const dstFilePath = path.join(os.tmpdir(), 'ldap-injection-methods.js')
177+
let ldapMethods
178+
179+
beforeEach(async () => {
180+
await agent.load('ldapjs')
181+
vulnerabilityReporter.clearCache()
182+
const ldapjs = require(`../../../../../../versions/ldapjs-promise@${promiseVersion}`).get()
183+
client = ldapjs.createClient({
184+
url: 'ldap://localhost:1389'
185+
})
186+
187+
fs.copyFileSync(srcFilePath, dstFilePath)
188+
ldapMethods = require(dstFilePath)
189+
190+
return client.bind(`cn=admin,${base}`, 'adminpassword')
191+
})
192+
193+
afterEach(async () => {
194+
fs.unlinkSync(dstFilePath)
195+
await client.unbind()
196+
})
197+
198+
describe('has vulnerability', () => {
199+
testThatRequestHasVulnerability(() => {
200+
const store = storage.getStore()
201+
const iastCtx = iastContextFunctions.getIastContext(store)
202+
203+
let filter = '(objectClass=*)'
204+
filter = newTaintedString(iastCtx, filter, 'param', 'Request')
205+
206+
return ldapMethods.executeSearch(client, base, filter)
207+
}, 'LDAP_INJECTION', {
208+
occurrences: 1,
209+
location: { path: 'ldap-injection-methods.js' }
210+
})
211+
})
212+
213+
describe('has no vulnerability', () => {
214+
testThatRequestHasNoVulnerability(() => {
215+
const filter = '(objectClass=*)'
216+
return ldapMethods.executeSearch(client, base, filter)
217+
}, 'LDAP_INJECTION')
218+
})
219+
})
220+
})
168221
})
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict'
2+
3+
function executeSearch (client, base, filter) {
4+
return client.search(base, filter)
5+
}
6+
7+
module.exports = { executeSearch }

packages/dd-trace/test/plugins/externals.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,16 @@
127127
"versions": ["6.1.0"]
128128
}
129129
],
130+
"ldapjs": [
131+
{
132+
"name": "ldapjs",
133+
"versions": [">= 2"]
134+
},
135+
{
136+
"name": "ldapjs-promise",
137+
"versions": [">=2"]
138+
}
139+
],
130140
"mariadb": [
131141
{
132142
"name": "mariadb",

0 commit comments

Comments
 (0)