semgrep
Express "tainted-sql-injection" is too broad in the strings it matches
Describe the bug
Using the rule javascript.express.security.injection.tainted-sql-string.tainted-sql-string, even simple log statements regarding the action taking place are matched. This is often problematic when commonly logged verbs are used which happen to coincide with the SQL verbs (Update, delete, etc...)
To Reproduce
const express = require('express')
const app = express()
const port = 3000
app.delete('/item', (req, res) => {
console.log("Will delete resource " + req.query.name)
// ...
res.sendStatus(200)
})
app.listen(port, () => console.log(`Example app listening at http://localhost:${port}`))
Because the log statement in the delete route contains the word "delete", it is assumed to be SQL, which it obviously isn't, and trips the warning.
Expected behavior I would expect this to be more careful about what strings are actually matched. I don't know what would be required to do this, since implementing a SQL parser for something like this is overkill, but perhaps some kind of heuristic based on where the string is used would work.
Priority How important is this to you?
- [ ] P0: blocking me from making progress
- [ ] P1: this will block me in the near future
- [x] P2: annoying but not blocking me
Screenshots N/A
Desktop (please complete the following information): N/A
Smartphone (please complete the following information): N/A
Additional context I was able to reproduce in the playground with Semgrep v1.20.0
Hey @ollien, thanks for filing this issue. I don't think we can really do that with the current state of Semgrep unfortunately.
There was an update to this rule yesterday (Apr 29th 2024), and after that we are also running into this rule being too broad. We have some code like this (I was able to repro the bug in the sandbox):
app.get(async (req, res) => {
const { foo } = req.body;
try {
// pass foo as param
createSomething(foo)
} catch (e) {
return res.status(500).json({
...errorModel,
message: `failed to create something with setting foo ${foo}`, //<-- error occurs here
});
}
});
If I change the word "create" in the message to "make" it doesn't throw Additionally if I comment out the createSomething(foo) line it also doesn't throw
