Express "tainted-sql-injection" is too broad in the strings it matches

[ollien]

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
• 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

[enncoded]

Hey @ollien, thanks for filing this issue. I don't think we can really do that with the current state of Semgrep unfortunately.

[Jacob-Steno]

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