Skip to content

Commit fe5bf31

Browse files
uurienuurien
authored
Remove AsyncResource from sequelize (#6125)
1 parent 94afa6f commit fe5bf31

File tree

4 files changed

+76
-30
lines changed

4 files changed

+76
-30
lines changed

‎packages/datadog-instrumentations/src/sequelize.js

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22

33
const {
44
channel,
5-
addHook,
6-
AsyncResource
5+
addHook
76
} = require('./helpers/instrument')
87

98
const shimmer = require('../../datadog-shimmer')
@@ -18,8 +17,6 @@ addHook({ name: 'sequelize', versions: ['>=4'] }, Sequelize => {
1817
return query.apply(this, arguments)
1918
}
2019

21-
const asyncResource = new AsyncResource('bound-anonymous-fn')
22-
2320
let dialect
2421
if (this.options && this.options.dialect) {
2522
dialect = this.options.dialect
@@ -33,22 +30,15 @@ addHook({ name: 'sequelize', versions: ['>=4'] }, Sequelize => {
3330
result = result[0]
3431
}
3532

36-
asyncResource.bind(function () {
37-
finishCh.publish({ result })
38-
}, this).apply(this)
33+
finishCh.runStores({ result }, () => {})
3934
}
4035

41-
return asyncResource.bind(function () {
42-
startCh.publish({
43-
sql,
44-
dialect
45-
})
46-
36+
return startCh.runStores({ sql, dialect }, () => {
4737
const promise = query.apply(this, arguments)
4838
promise.then(onFinish, () => { onFinish() })
4939

5040
return promise
51-
}, this).apply(this, arguments)
41+
})
5242
}
5343
})
5444

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,39 @@ class SqlInjectionAnalyzer extends StoredInjectionAnalyzer {
1818
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

21-
this.addSub(
21+
this.addBind(
2222
'datadog:sequelize:query:start',
2323
({ sql, dialect }) => this.getStoreAndAnalyze(sql, dialect.toUpperCase())
2424
)
2525
this.addSub('datadog:sequelize:query:finish', () => this.returnToParentStore())
2626

27-
this.addSub('datadog:pg:pool:query:start', ({ query }) => this.getStoreAndAnalyze(query.text, 'POSTGRES'))
27+
this.addSub('datadog:pg:pool:query:start', ({ query }) => this.setStoreAndAnalyze(query.text, 'POSTGRES'))
2828
this.addSub('datadog:pg:pool:query:finish', () => this.returnToParentStore())
2929

30-
this.addSub('datadog:mysql:pool:query:start', ({ sql }) => this.getStoreAndAnalyze(sql, 'MYSQL'))
30+
this.addSub('datadog:mysql:pool:query:start', ({ sql }) => this.setStoreAndAnalyze(sql, 'MYSQL'))
3131
this.addSub('datadog:mysql:pool:query:finish', () => this.returnToParentStore())
3232

3333
this.addSub('datadog:knex:raw:start', ({ sql, dialect: knexDialect }) => {
3434
const dialect = this.normalizeKnexDialect(knexDialect)
35-
this.getStoreAndAnalyze(sql, dialect)
35+
this.setStoreAndAnalyze(sql, dialect)
3636
})
3737
this.addSub('datadog:knex:raw:finish', () => this.returnToParentStore())
3838
}
3939

40+
setStoreAndAnalyze (query, dialect) {
41+
const store = this.getStoreAndAnalyze(query, dialect)
42+
43+
if (store) {
44+
storage('legacy').enterWith(store)
45+
}
46+
}
47+
4048
getStoreAndAnalyze (query, dialect) {
4149
const parentStore = storage('legacy').getStore()
4250
if (parentStore) {
4351
this.analyze(query, parentStore, dialect)
4452

45-
storage('legacy').enterWith({ ...parentStore, sqlAnalyzed: true, sqlParentStore: parentStore })
53+
return { ...parentStore, sqlAnalyzed: true, sqlParentStore: parentStore }
4654
}
4755
}
4856

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

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ describe('sql-injection-analyzer with sequelize', () => {
3333
return sequelize.authenticate()
3434
})
3535

36+
afterEach(() => {
37+
sequelize.close()
38+
})
39+
3640
testThatRequestHasVulnerability(() => {
3741
const store = storage('legacy').getStore()
3842
const iastCtx = iastContextFunctions.getIastContext(store)
@@ -45,9 +49,21 @@ describe('sql-injection-analyzer with sequelize', () => {
4549

4650
const externalFileContent = `'use strict'
4751
48-
module.exports = function (sequelize, sql) {
52+
function main (sequelize, sql) {
53+
return sequelize.query(sql)
54+
}
55+
56+
function doubleCall (sequelize, sql) {
57+
sequelize.query(sql)
4958
return sequelize.query(sql)
5059
}
60+
61+
async function doubleChainedCall (sequelize, sql) {
62+
await sequelize.query(sql)
63+
return sequelize.query(sql)
64+
}
65+
66+
module.exports = { main, doubleCall, doubleChainedCall }
5167
`
5268
testThatRequestHasVulnerability(() => {
5369
const filepath = path.join(os.tmpdir(), 'test-sequelize-sqli.js')
@@ -59,7 +75,7 @@ module.exports = function (sequelize, sql) {
5975
let sql = 'SELECT 1'
6076
sql = newTaintedString(iastCtx, sql, 'param', 'Request')
6177

62-
return require(filepath)(sequelize, sql)
78+
return require(filepath).main(sequelize, sql)
6379
}, 'SQL_INJECTION', {
6480
occurrences: 1,
6581
location: {
@@ -68,6 +84,36 @@ module.exports = function (sequelize, sql) {
6884
}
6985
})
7086

87+
testThatRequestHasVulnerability(() => {
88+
const filepath = path.join(os.tmpdir(), 'test-sequelize-sqli.js')
89+
fs.writeFileSync(filepath, externalFileContent)
90+
91+
const store = storage('legacy').getStore()
92+
const iastCtx = iastContextFunctions.getIastContext(store)
93+
94+
let sql = 'SELECT 1'
95+
sql = newTaintedString(iastCtx, sql, 'param', 'Request')
96+
97+
return require(filepath).doubleCall(sequelize, sql)
98+
}, 'SQL_INJECTION', {
99+
occurrences: 2
100+
})
101+
102+
testThatRequestHasVulnerability(() => {
103+
const filepath = path.join(os.tmpdir(), 'test-sequelize-sqli.js')
104+
fs.writeFileSync(filepath, externalFileContent)
105+
106+
const store = storage('legacy').getStore()
107+
const iastCtx = iastContextFunctions.getIastContext(store)
108+
109+
let sql = 'SELECT 1'
110+
sql = newTaintedString(iastCtx, sql, 'param', 'Request')
111+
112+
return require(filepath).doubleChainedCall(sequelize, sql)
113+
}, 'SQL_INJECTION', {
114+
occurrences: 2
115+
})
116+
71117
testThatRequestHasNoVulnerability(() => {
72118
return sequelize.query('SELECT 1')
73119
}, 'SQL_INJECTION')

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,20 @@ describe('sql-injection-analyzer', () => {
6464
sqlInjectionAnalyzer.configure(true)
6565

6666
it('should subscribe to mysql, mysql2 and pg start query channel', () => {
67-
expect(sqlInjectionAnalyzer._subscriptions).to.have.lengthOf(11)
67+
expect(sqlInjectionAnalyzer._subscriptions).to.have.lengthOf(10)
6868
expect(sqlInjectionAnalyzer._subscriptions[0]._channel.name).to.equals('apm:mysql:query:start')
6969
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')
71-
expect(sqlInjectionAnalyzer._subscriptions[3]._channel.name).to.equals('datadog:sequelize:query:start')
72-
expect(sqlInjectionAnalyzer._subscriptions[4]._channel.name).to.equals('datadog:sequelize:query:finish')
73-
expect(sqlInjectionAnalyzer._subscriptions[5]._channel.name).to.equals('datadog:pg:pool:query:start')
74-
expect(sqlInjectionAnalyzer._subscriptions[6]._channel.name).to.equals('datadog:pg:pool:query:finish')
75-
expect(sqlInjectionAnalyzer._subscriptions[7]._channel.name).to.equals('datadog:mysql:pool:query:start')
76-
expect(sqlInjectionAnalyzer._subscriptions[8]._channel.name).to.equals('datadog:mysql:pool:query:finish')
77-
expect(sqlInjectionAnalyzer._subscriptions[9]._channel.name).to.equals('datadog:knex:raw:start')
78-
expect(sqlInjectionAnalyzer._subscriptions[10]._channel.name).to.equals('datadog:knex:raw:finish')
71+
expect(sqlInjectionAnalyzer._subscriptions[3]._channel.name).to.equals('datadog:sequelize:query:finish')
72+
expect(sqlInjectionAnalyzer._subscriptions[4]._channel.name).to.equals('datadog:pg:pool:query:start')
73+
expect(sqlInjectionAnalyzer._subscriptions[5]._channel.name).to.equals('datadog:pg:pool:query:finish')
74+
expect(sqlInjectionAnalyzer._subscriptions[6]._channel.name).to.equals('datadog:mysql:pool:query:start')
75+
expect(sqlInjectionAnalyzer._subscriptions[7]._channel.name).to.equals('datadog:mysql:pool:query:finish')
76+
expect(sqlInjectionAnalyzer._subscriptions[8]._channel.name).to.equals('datadog:knex:raw:start')
77+
expect(sqlInjectionAnalyzer._subscriptions[9]._channel.name).to.equals('datadog:knex:raw:finish')
78+
79+
expect(sqlInjectionAnalyzer._bindings).to.have.lengthOf(1)
80+
expect(sqlInjectionAnalyzer._bindings[0]._channel.name).to.equals('datadog:sequelize:query:start')
7981
})
8082

8183
it('should not detect vulnerability when no query', () => {

0 commit comments

Comments
 (0)