-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Closed
Labels
Description
According to the mysql documentation (check sentence starting with The third form
)
connection.query({
sql: 'SELECT * FROM `books` WHERE `author` = ?',
timeout: 40000, // 40s
values: ['David']
}, function (error, results, fields) {
// error will be an Error if one occurred during the query
// results will contain the results of the query
// fields will contain information about the returned results fields (if any)
});
is a valid way to perform a query.
However, for the MySQL package - CodeQL seems to only cover the first and second design patterns (see implementation). Therefore on the following file it only reports one vulnerability.
var mysql = require('mysql');
var pool = mysql.createPool({
connectionLimit: 10,
host: 'example.org',
user: 'bob',
password: 'secret',
database: 'my_db'
});
const app = require("express")();
app.get("search", function handler(req, res) {
let temp = req.params.value;
pool.getConnection(function(err, connection) {
connection.query({
sql: 'SELECT * FROM `books` WHERE `author` = ?', // GOOD
values: [temp]
}, function(error, results, fields) {});
});
pool.getConnection(function(err, connection) {
connection.query({
sql: 'SELECT * FROM `books` WHERE `author` = ' + temp, // BAD
}, function(error, results, fields) {});
});
pool.getConnection(function(err, connection) {
connection.query('SELECT * FROM `books` WHERE `author` = ' + temp, // BAD
function(error, results, fields) {});
});
});
So additionally if the first argument is an object, we should send the sql
attribute of that object as a query too?
Let me know if this looks good. I can also submit a PR adding this and a corresponding test in the query-tests folder.