Skip to content

Commit 9bad2bb

Browse files
roschaeferroschaefer
authored
Merge pull request #16 from Human-Connection/14-patch-sql-injections
Escape all sql statements to avoid sql injections
2 parents 0202c71 + 9b14391 commit 9bad2bb

3 files changed

Lines changed: 122 additions & 118 deletions

File tree

‎core/db.js‎

Lines changed: 50 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ exports.toggleEntryStatus = function(id, state, callback){
2121
// 1 = confirmed | 2 = removed
2222
let sql;
2323
if(state === '1' || state === '2'){
24-
sql = "UPDATE entries SET status = "+state+" WHERE id = "+id;
24+
sql = "UPDATE entries SET status = ? WHERE id = ?";
2525
}else{
26-
callback({}, false);
27-
return;
26+
callback({}, false);
27+
return;
2828
}
2929

3030
// make the query
31-
connection.query(sql, function(err, results) {
31+
connection.query(sql, [state, id], function(err, results) {
3232
connection.release();
3333
if(err) { callback(results, true); return; }
3434
callback(results, false);
@@ -39,12 +39,11 @@ exports.toggleEntryStatus = function(id, state, callback){
3939
exports.isValidApiKey = function(secret, callback){
4040
pool.getConnection(function(err, connection) {
4141
if(err) { console.log(err); callback(true); return; }
42-
let sql = "SELECT valid from apikeys WHERE secret = '"+secret+"';";
4342

44-
console.log(sql);
43+
let sql = "SELECT valid from apikeys WHERE secret = '?';";
4544

4645
// make the query
47-
connection.query(sql, function(err, results) {
46+
connection.query(sql, [secret], function(err, results) {
4847
connection.release();
4948
if(err) { callback(results, true); return; }
5049
callback(results, false);
@@ -57,15 +56,13 @@ exports.getEntries = function(limit, offset, active, callback){
5756
if(err) { console.log(err); callback(true); return; }
5857
let sql = '';
5958
if(active === '0'){
60-
sql = "SELECT * FROM entries ORDER BY ID DESC LIMIT "+limit+" OFFSET "+offset+";";
59+
sql = "SELECT * FROM entries ORDER BY ID DESC LIMIT ? OFFSET ?;";
6160
}else{
62-
sql = "SELECT * FROM entries WHERE email_confirmed = 1 AND status = 1 ORDER BY ID DESC LIMIT "+limit+" OFFSET "+offset+";";
61+
sql = "SELECT * FROM entries WHERE email_confirmed = 1 AND status = 1 ORDER BY ID DESC LIMIT ? OFFSET ?;";
6362
}
6463

65-
console.log(sql);
66-
6764
// make the query
68-
connection.query(sql, function(err, results) {
65+
connection.query(sql, [limit, offset], function(err, results) {
6966
connection.release();
7067
if(err) { callback(results, true); return; }
7168
callback(results, false);
@@ -75,11 +72,12 @@ exports.getEntries = function(limit, offset, active, callback){
7572

7673
exports.getUserByHash = function(hash, callback){
7774
pool.getConnection(function(err, connection) {
78-
if(err) { console.log(err); callback(true); return; }
79-
let sql = "SELECT email, firstname from entries WHERE confirm_key = '"+hash+"';";
75+
if (err) { console.log(err); callback(true); return; }
76+
77+
let sql = "SELECT email, firstname from entries WHERE confirm_key = '?';";
8078

8179
// make the query
82-
connection.query(sql, function(err, results) {
80+
connection.query(sql, [hash], function(err, results) {
8381
connection.release();
8482
if(err) { callback(results, true); return; }
8583
callback(results, false);
@@ -90,12 +88,12 @@ exports.getUserByHash = function(hash, callback){
9088
exports.verifyEntry = function(hash, callback){
9189
pool.getConnection(function(err, connection) {
9290
if(err) { console.log(err); callback(true); return; }
93-
let sql = "UPDATE entries set email_confirmed = 1, confirmed_at = "+moment().valueOf()+" WHERE";
94-
sql += " confirm_key = '"+hash+"' AND";
95-
sql += " confirmed_at is null;";
91+
92+
let sql = "UPDATE entries set email_confirmed = 1, confirmed_at = ? "
93+
+ "WHERE confirm_key = '?' AND confirmed_at is null;";
9694

9795
// make the query
98-
connection.query(sql, function(err, results) {
96+
connection.query(sql, [moment().valueOf(), hash], function(err, results) {
9997
connection.release();
10098
if(err || results.affectedRows < 1) { callback(results, true); return; }
10199
callback(results, false);
@@ -106,6 +104,7 @@ exports.verifyEntry = function(hash, callback){
106104
exports.getCount = function(callback){
107105
pool.getConnection(function(err, connection) {
108106
if(err) { console.log(err); callback(true); return; }
107+
109108
let sql = "SELECT count(*) as cnt FROM entries WHERE email_confirmed > 0 AND status < 2 AND country != '';";
110109

111110
// make the query
@@ -122,45 +121,51 @@ exports.saveEntry = function(fields, callback){
122121
if(err) { console.log(err); callback(true); return; }
123122
let data = prepareEntry(fields);
124123

125-
let sqle = "SELECT count(*) as cnt FROM entries WHERE email = '"+data.email+"';";
126-
connection.query(sqle, function(err, results) {
124+
let sqlEmailExists = "SELECT count(*) as cnt FROM entries WHERE email = '?';";
125+
connection.query(sqlEmailExists, [data.email], function(err, results) {
127126
if(!err) {
128127
if(results[0]['cnt'] > 0){
129128
callback(true);
130129
return;
131130
}else{
132-
let sql = "INSERT INTO entries (firstname, lastname, email, country, message, anon, ipv4, image, created_at, updated_at, confirm_key, beta, newsletter, pax) VALUES (";
133-
sql += "'"+ data.firstname + "', ";
134-
sql += "'"+ data.lastname + "', ";
135-
sql += "'"+ data.email + "', ";
136-
sql += "'"+ data.country + "', ";
137-
sql += "'"+ data.message + "', ";
138-
sql += data.anon + ", ";
139-
sql += "'"+ data.ipv4 + "', ";
140-
sql += "'"+ data.image + "', ";
141-
sql += data.created_at + ", ";
142-
sql += data.updated_at + ", ";
143-
sql += "'"+ data.randomHash + "', ";
144-
sql += data.beta + ", ";
145-
sql += data.newsletter + ", ";
146-
sql += data.pax + ");";
131+
let sql = "INSERT INTO entries (firstname, lastname, email, country, message, anon, ipv4, image, "
132+
+ "created_at, updated_at, confirm_key, beta, newsletter, pax) "
133+
+ "VALUES ('?', '?', '?', '?', '?', ?, '?', '?', ?, ?, '?', ?, ?, ?);";
147134

148135
// run the query
149-
connection.query(sql, function(err, results) {
150-
connection.release();
151-
if(err) { callback(true); return; }
152-
callback(false, results);
153-
});
136+
connection.query(
137+
sql,
138+
[
139+
data.firstname,
140+
data.lastname,
141+
data.email,
142+
data.country,
143+
data.message,
144+
data.anon,
145+
data.ipv4,
146+
data.image,
147+
data.created_at,
148+
data.updated_at,
149+
data.randomHash,
150+
data.beta,
151+
data.newsletter,
152+
data.pax,
153+
],
154+
function(err, results) {
155+
connection.release();
156+
if(err) { callback(true); return; }
157+
callback(false, results);
158+
}
159+
);
154160
}
155161
}
156162
});
157163
});
158164
};
159165

160-
/*
161-
* most fields get sanitized and escaped by node-mysql
166+
/**
162167
* this function is to prevent application errors
163-
**/
168+
*/
164169
function prepareEntry(data){
165170
let now = moment().valueOf();
166171

@@ -171,4 +176,4 @@ function prepareEntry(data){
171176
data["updated_at"] = now;
172177

173178
return data;
174-
}
179+
}

‎core/entryController.js‎

Lines changed: 71 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -13,63 +13,63 @@ exports.getAll = function(req, res) {
1313
active = req.query.isActive || 1;
1414
db.getEntries(limit, offset, active, function(results, err){
1515
if(!err){
16-
// format output where required
17-
let out = [];
18-
results.forEach(function(item){
19-
let obj = {};
20-
21-
obj.id = item.id;
22-
obj.email = item.email;
23-
obj.firstname = item.firstname;
24-
obj.lastname = item.lastname;
25-
obj.message = item.message;
26-
obj.country = item.country;
27-
obj.ipv4 = item.ipv4;
28-
obj.ipv6 = item.ipv6;
29-
obj.email_confirmed = item.email_confirmed;
30-
obj.confirm_key = item.confirm_key;
31-
obj.status = item.status;
32-
obj.anon = item.anon;
33-
obj.created_at = item.created_at;
34-
obj.updated_at = item.updated_at;
35-
obj.confirmed_at = item.confirmed_at;
36-
37-
if(item.image !== ''){
38-
obj.image = "https://" + req.hostname + "/uploads/"+item.image;
39-
}else{
40-
obj.image = '';
41-
}
42-
43-
44-
out.push(obj);
45-
});
46-
47-
res.status(200).json({
48-
success : true,
49-
results : out,
50-
totalCount : results.length,
51-
page : offset
52-
});
16+
// format output where required
17+
let out = [];
18+
results.forEach(function(item){
19+
let obj = {};
20+
21+
obj.id = item.id;
22+
obj.email = item.email;
23+
obj.firstname = item.firstname;
24+
obj.lastname = item.lastname;
25+
obj.message = item.message;
26+
obj.country = item.country;
27+
obj.ipv4 = item.ipv4;
28+
obj.ipv6 = item.ipv6;
29+
obj.email_confirmed = item.email_confirmed;
30+
obj.confirm_key = item.confirm_key;
31+
obj.status = item.status;
32+
obj.anon = item.anon;
33+
obj.created_at = item.created_at;
34+
obj.updated_at = item.updated_at;
35+
obj.confirmed_at = item.confirmed_at;
36+
37+
if(item.image !== ''){
38+
obj.image = "https://" + req.hostname + "/uploads/"+item.image;
39+
}else{
40+
obj.image = '';
41+
}
42+
43+
44+
out.push(obj);
45+
});
46+
47+
res.status(200).json({
48+
success : true,
49+
results : out,
50+
totalCount : results.length,
51+
page : offset
52+
});
5353
}else{
5454
res.status(400).json({success : false, message : "error"});
5555
}
5656
});
5757
};
5858

5959
exports.toggleStatus = function(req, res){
60-
let form = new formidable.IncomingForm();
61-
form.parse(req, function (err, fields) {
62-
console.log(fields.id)
63-
if(fields && fields.id !== undefined){
64-
db.toggleEntryStatus(fields.id, fields.state, function(results, err){
65-
if(!err){
66-
res.status(200).json({success : true, message : "toggled status"});
67-
}else{
68-
res.status(400).json({success : false, message : "error"});
69-
}
70-
})
71-
}
72-
});
60+
let form = new formidable.IncomingForm();
61+
form.parse(req, function (err, fields) {
62+
63+
if(fields && fields.id !== undefined){
64+
db.toggleEntryStatus(fields.id, fields.state, function(results, err){
65+
if(!err){
66+
res.status(200).json({success : true, message : "toggled status"});
67+
}else{
68+
res.status(400).json({success : false, message : "error"});
69+
}
70+
})
71+
}
72+
});
7373
};
7474

7575
exports.getCount = function(req, res) {
@@ -87,14 +87,14 @@ exports.getCount = function(req, res) {
8787
exports.verifyEntry = function(req, res) {
8888
db.verifyEntry(req.params.k, function(results, err){
8989
if(!err){
90-
db.getUserByHash(req.params.k, function(results, err){
91-
if(!err){
92-
mailer.sendVerifySuccess({email : results[0].email, firstname : results[0].firstname});
93-
res.redirect('https://human-connection.org/uhr-des-wandels/?ns=t');
94-
}
95-
});
90+
db.getUserByHash(req.params.k, function(results, err){
91+
if(!err){
92+
mailer.sendVerifySuccess({email : results[0].email, firstname : results[0].firstname});
93+
res.redirect('https://human-connection.org/uhr-des-wandels/?ns=t');
94+
}
95+
});
9696
}else{
97-
res.redirect('https://human-connection.org/uhr-des-wandels/?ns=f');
97+
res.redirect('https://human-connection.org/uhr-des-wandels/?ns=f');
9898
}
9999
});
100100
};
@@ -180,29 +180,29 @@ exports.createEntry = function(req, res) {
180180
fields["ipv4"] = req.ip;
181181

182182
if(hasFile){
183-
fields["image"] = hasFile ? files[0].path.replace('uploads/', '') : '';
183+
fields["image"] = hasFile ? files[0].path.replace('uploads/', '') : '';
184184

185-
let newFile = resize(files[0].path, 200, 200);
186-
fs.unlinkSync(files[0].path);
187-
newFile.toFile('uploads/'+fields["image"], (err, info) => {});
185+
let newFile = resize(files[0].path, 200, 200);
186+
fs.unlinkSync(files[0].path);
187+
newFile.toFile('uploads/'+fields["image"], (err, info) => {});
188188
}else{
189-
fields["image"] = '';
189+
fields["image"] = '';
190190
}
191191

192192
let hash = crypto.randomBytes(32).toString('hex');
193193
fields["randomHash"] = hash;
194194

195195
// save to db
196196
db.saveEntry(fields, function(err, results){
197-
if(!err){
198-
// send verification email
199-
mailer.sendVerificationMail(hash, {email : fields.email, firstname : fields.firstname});
200-
201-
res.status(200).json({success : true});
202-
}else{
203-
res.status(400).json({success : false, message : "error"});
204-
}
197+
if(!err){
198+
// send verification email
199+
mailer.sendVerificationMail(hash, {email : fields.email, firstname : fields.firstname});
200+
201+
res.status(200).json({success : true});
202+
}else{
203+
res.status(400).json({success : false, message : "error"});
204+
}
205205
});
206206
}
207207
});
208-
};
208+
};

‎core/restapi.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ module.exports = function(app) {
1111
app.route('/entries/verify/:k').get(entryController.verifyEntry);
1212

1313
// intercept request and check for api key
14-
1514
app.use((req, res, next) => {
1615
let apiKey = req.get('API-Key');
1716
db.isValidApiKey(apiKey, function(results, err){
@@ -27,4 +26,4 @@ module.exports = function(app) {
2726
app.route('/entries').post(entryController.createEntry);
2827
app.route('/entries/toggle').post(entryController.toggleStatus);
2928
app.route('/entries').get(entryController.getAll);
30-
};
29+
};

0 commit comments

Comments
 (0)