Reputation: 7114
I've been developing an "Employee leave management" web app project for our internal use using node.js with express and ejs template. Now, my employer wants me to make the app accessible through internet and I'm worried about SQL injection.
Let's say I have a button like this in html:
<a href="/edit/<%= ReqID %>">Edit</a>
This will GET
from index.js
file:
const { edit } = require("./request");
app.get("/edit/:ReqID", edit);
This will then go to module edit
in request.js
file:
module.exports = {
edit: (req, res) => {
let ReqID= req.params.ReqID;
let squery = `SELECT * FROM table1 WHERE ReqID="${ReqID}";
SELECT * FROM table2 WHERE ReqID="${ReqID}";`;
db.query(squery, function (err, result) {
if (err) {
return res.status(500).send(err);
}
res.render("edit.ejs", {
srecords1: result[0],
srecords2: result[1]
})
})
}
}
There might be two or more queries in there and I'm using mysql
driver for node.js with multipleStatements: true
and I'm aware of warning "Support for multiple statements is disabled for security reasons (it allows for SQL injection attacks if values are not properly escaped)." This will return something like http://localhost:port/edit/reqid
on the browser address box. I saw a video from youtube that says SQL Injection can be done through the browser's address box like http://localhost:port/edit/reqid;";SELECT * FROM users;
so I did that and for sure I can see that syntax being send to the server. So I follow the suggestion in the video to do a placeholder like this:
module.exports = {
edit: (req, res) => {
let ReqID= req.params.ReqID;
let squery = `SELECT * FROM table1 WHERE ReqID= ?;
SELECT * FROM table2 WHERE ReqID= ?;`;
db.query(squery, [ReqID, ReqID], function (err, result) {
if (err) {
return res.status(500).send(err);
}
res.render("edit.ejs", {
srecords1: result[0],
srecords2: result[1]
})
})
}
}
Then I try the extreme http://localhost:port/edit/reqid;";DELETE FROM users;
and http://localhost:port/edit/reqid;";DROP TABLE users;
separately and it works! First it deletes data from users
tble and for sure the second drop table command also worked. After the first attempt, I refresh the browser with the same sql injection syntax and I've got this message:
{"code":"ER_BAD_TABLE_ERROR","errno":1051,"sqlMessage":"Unknown table 'users'","sqlState":"42S02","index":1,"sql":"SELECT * FROM table1 WHERE ReqID= "ReqID;";drop table users;";SELECT * FROM table1 WHERE ReqID= "ReqID;";drop table users;";"}
So, the table users
clearly have been dropped from the database.
Update:
I did further testing based on the information I gained from this answer and I did something like this:
module.exports = {
edit: (req, res) => {
let ReqID= req.params.ReqID;
db.query(`SELECT * FROM table1 WHERE ReqID= ?; SELECT * FROM table2 WHERE ReqID= ?;` , [ReqID, ReqID], function (err, result) {
if (err) {
return res.status(500).send(err);
}
res.render("edit.ejs", {
srecords1: result[0],
srecords2: result[1]
})
})
}
}
Then I re-test with multiple variation of http://localhost:port/edit/reqid;";DROP TABLE users;
(double quote in between)
http://localhost:port/edit/reqid;';DROP TABLE users;
(single quote in between) etc. and it doesn't seem to be dropping the table anymore. However, I still see the statement being sent to the server so I'm still wary of the DROP
syntax being effective somehow.
Update 2:
Note: Fortunately, the deployment has been delayed and I have more time to sort out the issue.
After researching for a while, taking the comments into consideration and testing multiple method, I came up with this structure:
function(req, res) {
let dcode = [req.body.dcode];
let query1 =`SELECT col1, col2 FROM table1 WHERE DCode=?`;
db.query(query1, dcode, function(err, result_1) {
if (err) {
return res.status(500).send(err);
}
let query2 =`SELECT col1, col2 FROM table2 WHERE DCode=?`;
db.query(query2, dcode, function(err, result_2) {
if (err) {
return res.status(500).send(err);
}
res.render("login.ejs", {
result1: result_1,
result2: result_2
});
});
});
}
Which is simple enough and no major change to my current codes. Would this be sufficient to prevent SQL injection in node.js?
Upvotes: 1
Views: 573
Reputation: 1343
Here are a few suggestions that might help:
Select * from table where id = ${value}
. SQL injections will happen - 100%!. Instead you should use build in driver defense mechanism. Like this: query('Select * from table where id = ?', [value])
. This should prevent SQL injection.Upvotes: 2
Reputation: 142296
Allowing multi-statement strings, itself, invites SQL injection. So, avoid it.
Plan A:
Consider ending an array (perhaps in JSON) to the server; let it then execute each statement, and return an array of resultsets.
But it would be simpler to simply issue the statements one at a time.
(If the client and server are far apart, the one-statement-at-a-time method may cause a noticeable latency.)
Plan B:
Build suitable Stored procedures for any multi-statement needs. This, where practical, avoids multi-statement calls. And avoids latency issues (usually).
Upvotes: 2