Skip to content

Commit

Permalink
Fix mongoose IAST exec with callback (#4045)
Browse files Browse the repository at this point in the history 
* Add test for fix

* Another test, expecting to detect the vuln

* fix mongoose instru not supporting deferred callback

* add more tests

* make sure callback is not wrapped twice

---------

Co-authored-by: simon-id <simon.id@datadoghq.com>
  • Loading branch information
@uurien @simon-id
uurien and simon-id authored Feb 12, 2024
1 parent 7d496a5 commit 06ef74d
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 77 deletions.
33 changes: 23 additions & 10 deletions packages/datadog-instrumentations/src/mongoose.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,26 @@ addHook({
})

let callbackWrapped = false
const lastArgumentIndex = arguments.length - 1

if (typeof arguments[lastArgumentIndex] === 'function') {
// is a callback, wrap it to execute finish()
shimmer.wrap(arguments, lastArgumentIndex, originalCb => {
return function () {
finish()
const wrapCallbackIfExist = (args) => {
const lastArgumentIndex = args.length - 1

return originalCb.apply(this, arguments)
}
})
if (typeof args[lastArgumentIndex] === 'function') {
// is a callback, wrap it to execute finish()
shimmer.wrap(args, lastArgumentIndex, originalCb => {
return function () {
finish()

return originalCb.apply(this, arguments)
}
})

callbackWrapped = true
callbackWrapped = true
}
}

wrapCallbackIfExist(arguments)

return asyncResource.runInAsyncScope(() => {
startCh.publish({
filters,
Expand All @@ -106,8 +111,16 @@ addHook({
if (!callbackWrapped) {
shimmer.wrap(res, 'exec', originalExec => {
return function wrappedExec () {
if (!callbackWrapped) {
wrapCallbackIfExist(arguments)
}

const execResult = originalExec.apply(this, arguments)

if (callbackWrapped || typeof execResult?.then !== 'function') {
return execResult
}

// wrap them method, wrap resolve and reject methods
shimmer.wrap(execResult, 'then', originalThen => {
return function wrappedThen () {
Expand Down
216 changes: 149 additions & 67 deletions ...trace/test/appsec/iast/analyzers/nosql-injection-mongodb-analyzer.mongoose.plugin.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,91 +52,173 @@ describe('nosql injection detection in mongodb - whole feature', () => {

prepareTestServerForIastInExpress('Test with mongoose', expressVersion,
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
testThatRequestHasVulnerability({
fn: async (req, res) => {
Test.find({
name: req.query.key,
value: [1, 2,
'value',
false, req.query.key]
}).then(() => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
})

testThatRequestHasVulnerability({
fn: async (req, res) => {
Test.find({
name: {
child: [req.query.key]
}
}).then(() => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
})

testThatRequestHasVulnerability({
testDescription: 'should have NOSQL_MONGODB_INJECTION vulnerability in correct file and line',
fn: async (req, res) => {
const filter = {
name: {
child: [req.query.key]
}
describe('using promises', () => {
testThatRequestHasVulnerability({
fn: async (req, res) => {
Test.find({
name: req.query.key,
value: [1, 2,
'value',
false, req.query.key]
}).then(() => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
require(tmpFilePath)(Test, filter, () => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
},
occurrences: {
occurrences: 1,
location: {
path: vulnerableMethodFilename,
line: 4
})

testThatRequestHasVulnerability({
fn: async (req, res) => {
Test.find({
name: {
child: [req.query.key]
}
}).then(() => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
}
})
})

if (semver.satisfies(specificMongooseVersion, '>=6')) {
testThatRequestHasNoVulnerability({
testDescription: 'should not have NOSQL_MONGODB_INJECTION vulnerability with mongoose.sanitizeFilter',
testThatRequestHasVulnerability({
testDescription: 'should have NOSQL_MONGODB_INJECTION vulnerability using promise in exec method',
fn: async (req, res) => {
const filter = mongoose.sanitizeFilter({
Test.find({
name: {
child: [req.query.key]
}
}).exec().then(() => {
res.end()
})
Test.find(filter).then(() => {
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
})

testThatRequestHasVulnerability({
testDescription: 'should have NOSQL_MONGODB_INJECTION vulnerability in correct file and line',
fn: async (req, res) => {
const filter = {
name: {
child: [req.query.key]
}
}
require(tmpFilePath)(Test, filter, () => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
},
occurrences: {
occurrences: 1,
location: {
path: vulnerableMethodFilename,
line: 4
}
}
})
}

testThatRequestHasNoVulnerability(async (req, res) => {
Test.find({
name: 'test'
}).then(() => {
res.end()
if (semver.satisfies(specificMongooseVersion, '>=6')) {
testThatRequestHasNoVulnerability({
testDescription: 'should not have NOSQL_MONGODB_INJECTION vulnerability with mongoose.sanitizeFilter',
fn: async (req, res) => {
const filter = mongoose.sanitizeFilter({
name: {
child: [req.query.key]
}
})
Test.find(filter).then(() => {
res.end()
})
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
})
}

testThatRequestHasNoVulnerability(async (req, res) => {
Test.find({
name: 'test'
}).then(() => {
res.end()
})
}, 'NOSQL_MONGODB_INJECTION')
})

if (semver.satisfies(specificMongooseVersion, '<7')) {
describe('using callbacks', () => {
testThatRequestHasNoVulnerability(async (req, res) => {
try {
Test.find({
name: 'test'
}).exec(() => {
res.end()
})
} catch (e) {
res.writeHead(500)
res.end()
}
}, 'NOSQL_MONGODB_INJECTION')

testThatRequestHasVulnerability({
textDescription: 'should have NOSQL_MONGODB_INJECTION vulnerability using callback in exec',
fn: async (req, res) => {
try {
Test.find({
name: req.query.key,
value: [1, 2,
'value',
false, req.query.key]
}).exec(() => {
res.end()
})
} catch (e) {
res.writeHead(500)
res.end()
}
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
})

testThatRequestHasVulnerability({
textDescription: 'should have NOSQL_MONGODB_INJECTION vulnerability using callback in find',
fn: async (req, res) => {
try {
Test.find({
name: req.query.key,
value: [1, 2,
'value',
false, req.query.key]
}, () => {
res.end()
})
} catch (e) {
res.writeHead(500)
res.end()
}
},
vulnerability: 'NOSQL_MONGODB_INJECTION',
makeRequest: (done, config) => {
axios.get(`http://localhost:${config.port}/?key=value`).catch(done)
}
})
})
}, 'NOSQL_MONGODB_INJECTION')
}
})
})
})
Expand Down

0 comments on commit 06ef74d

Please sign in to comment.