Skip to content

Commit ec706ad

Browse files
authored
Fix iast mysql2 (#5847)
1 parent e5dcebc commit ec706ad

File tree

4 files changed

+56
-17
lines changed

4 files changed

+56
-17
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class SqlInjectionAnalyzer extends StoredInjectionAnalyzer {
1515

1616
onConfigure () {
1717
this.addSub('apm:mysql:query:start', ({ sql }) => this.analyze(sql, undefined, 'MYSQL'))
18-
this.addSub('apm:mysql2:query:start', ({ sql }) => this.analyze(sql, undefined, 'MYSQL'))
18+
this.addSub('datadog:mysql2:outerquery:start', ({ sql }) => this.analyze(sql, undefined, 'MYSQL'))
1919
this.addSub('apm:pg:query:start', ({ query }) => this.analyze(query.text, undefined, 'POSTGRES'))
2020

2121
this.addSub(
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict'
2+
3+
module.exports = function vulnerableMethod (connection, sql) {
4+
return new Promise((resolve, reject) => {
5+
connection.query(sql, function (err) {
6+
if (err) {
7+
reject(err)
8+
} else {
9+
resolve()
10+
}
11+
})
12+
})
13+
}

packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.mysql2.plugin.spec.js

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

3+
const path = require('path')
4+
const os = require('os')
5+
const fs = require('fs')
6+
const { assert } = require('chai')
37
const { prepareTestServerForIast } = require('../utils')
48
const { storage } = require('../../../../../datadog-core')
59
const iastContextFunctions = require('../../../../src/appsec/iast/iast-context')
@@ -9,6 +13,7 @@ const vulnerabilityReporter = require('../../../../src/appsec/iast/vulnerability
913
describe('sql-injection-analyzer with mysql2', () => {
1014
let mysql2
1115
let connection
16+
1217
withVersions('mysql2', 'mysql2', version => {
1318
prepareTestServerForIast('mysql2', (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
1419
beforeEach(() => {
@@ -26,22 +31,43 @@ describe('sql-injection-analyzer with mysql2', () => {
2631
connection.end(done)
2732
})
2833

29-
describe('has vulnerability', () => {
34+
describe('has vulnerability in the right file/line', () => {
35+
let tmpFilePath
36+
const vulnerableMethodFilename = 'mysql2-vulnerable-method.js'
37+
38+
beforeEach(() => {
39+
tmpFilePath = path.join(os.tmpdir(), vulnerableMethodFilename)
40+
41+
try {
42+
fs.unlinkSync(tmpFilePath)
43+
} catch (e) {
44+
// ignore the error
45+
}
46+
const src = path.join(__dirname, 'resources', vulnerableMethodFilename)
47+
48+
fs.copyFileSync(src, tmpFilePath)
49+
})
50+
51+
afterEach(() => {
52+
try {
53+
fs.unlinkSync(tmpFilePath)
54+
} catch (e) {
55+
// ignore the error
56+
}
57+
})
58+
3059
testThatRequestHasVulnerability(() => {
31-
return new Promise((resolve, reject) => {
32-
const store = storage('legacy').getStore()
33-
const iastCtx = iastContextFunctions.getIastContext(store)
34-
let sql = 'SELECT 1'
35-
sql = newTaintedString(iastCtx, sql, 'param', 'Request')
36-
connection.query(sql, function (err) {
37-
if (err) {
38-
reject(err)
39-
} else {
40-
resolve()
41-
}
42-
})
43-
})
44-
}, 'SQL_INJECTION')
60+
const store = storage('legacy').getStore()
61+
const iastCtx = iastContextFunctions.getIastContext(store)
62+
let sql = 'SELECT 1'
63+
sql = newTaintedString(iastCtx, sql, 'param', 'Request')
64+
const vulnerableMethod = require(tmpFilePath)
65+
66+
return vulnerableMethod(connection, sql)
67+
}, 'SQL_INJECTION', 1, function ([vulnerability]) {
68+
assert.isTrue(vulnerability.location.path.endsWith(vulnerableMethodFilename))
69+
assert.equal(vulnerability.location.line, 5)
70+
})
4571
})
4672

4773
describe('has no vulnerability', () => {

packages/dd-trace/test/appsec/iast/analyzers/sql-injection-analyzer.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe('sql-injection-analyzer', () => {
6666
it('should subscribe to mysql, mysql2 and pg start query channel', () => {
6767
expect(sqlInjectionAnalyzer._subscriptions).to.have.lengthOf(11)
6868
expect(sqlInjectionAnalyzer._subscriptions[0]._channel.name).to.equals('apm:mysql:query:start')
69-
expect(sqlInjectionAnalyzer._subscriptions[1]._channel.name).to.equals('apm:mysql2:query:start')
69+
expect(sqlInjectionAnalyzer._subscriptions[1]._channel.name).to.equals('datadog:mysql2:outerquery:start')
7070
expect(sqlInjectionAnalyzer._subscriptions[2]._channel.name).to.equals('apm:pg:query:start')
7171
expect(sqlInjectionAnalyzer._subscriptions[3]._channel.name).to.equals('datadog:sequelize:query:start')
7272
expect(sqlInjectionAnalyzer._subscriptions[4]._channel.name).to.equals('datadog:sequelize:query:finish')

0 commit comments

Comments
 (0)