I almost finished my register but I found some vulnerability.
Whenever I add: ?Smith into my register form, I receive the error:
Error: ER_PARSE_ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '??Smith' at line 1
Vulnerable line (line 86):
connection.query('SELECT * FROM penguins WHERE username = ?' + username, function(err, rows) { + connection.escape(email); + connection.escape(username); + connection.escape(req.body.email); + connection.escape(req.body.nickname);
I tried to escape it with connection.escape(); but it didn't work. Email, username and password are all vulnerable inputs.
My passport strategy:
const dateTime = new Date().getTime();
const timestamp = Math.floor(dateTime / 1000);
var mysql = require('mysql');
var LocalStrategy = require('passport-local').Strategy;
var connection = mysql.createConnection({
host: 'localhost',
user: 'root',
password: 'root'
});
connection.config.queryFormat = function (query, values) {
if (!values) return query;
return query.replace(/\:%'"=-?(\w+)/g, function (txt, key) {
if (values.hasOwnProperty(key)) {
return this.escape(values[key]);
}
return txt;
}.bind(this));
};
connection.query('USE kitsune');
// expose this function to our app using module.exports
module.exports = function(passport) {
// =========================================================================
// passport session setup ==================================================
// =========================================================================
// required for persistent login sessions
// passport needs ability to serialize and unserialize users out of session
// used to serialize the user for the session
passport.serializeUser(function(user, done) {
done(null, user.id);
});
// used to deserialize the user
passport.deserializeUser(function(id, done) {
connection.query('SELECT * FROM penguins WHERE id = ?' + id, function(err, rows) { + connection.escape(id);
connection.escape(id);
id.toString();
done(err, rows[0]);
});
});
// =========================================================================
// LOCAL SIGNUP ============================================================
// =========================================================================
// we are using named strategies since we have one for login and one for signup
// by default, if there was no name, it would just be called 'local'
passport.use('local-signup', new LocalStrategy({
// by default, local strategy uses username and password, we will override with email
usernameField: 'username',
passwordField: 'password',
gameusernameField: 'username',
nicknameField: 'nickname',
passReqToCallback: true // allows us to pass back the entire request to the callback
},
function(req, username, password, done) {
// here you read from req
const email = req.body.email
const nickname = req.body.nickname
const inventory = '%1'; // This is what the user gets on register. You can set this to anything that you want like: %1%2%3%4%5%6%7%8%9%10%11%12%13%14%15%16
const moderator = '0';
//const registrationdate = timestamp
const igloo = '1';
const igloos = '1';
console.log("Inventory is set to: " + inventory);
console.log("Moderator is set to: " + moderator);
console.log("Igloo is set to: " + igloo);
console.log("Igloos is set to: " + igloos);
passport.serializeUser(function(username, done) {
done(null, username);
});
// find a user whose email is the same as the forms email
// we are checking to see if the user trying to login already exists
connection.query('SELECT * FROM penguins WHERE username = ?' + username, function(err, rows) { + connection.escape(email); + connection.escape(username); + connection.escape(req.body.email); + connection.escape(req.body.nickname);
username.toString();
password.toString();
email.toString();
console.log(rows);
console.log("above row object");
if (err) return done(err);
if (rows.length) {
return done(null, false, req.flash('signupMessage', 'That username is already taken.'));
} else {
// if there is no user with that email
// create the user
var newUserMysql = new Object();
newUserMysql.igloos = igloos;
newUserMysql.igloo = igloo;
//newUserMysql.registrationdate = registrationdate;
newUserMysql.moderator = moderator;
newUserMysql.inventory = inventory;
newUserMysql.email = email;
newUserMysql.password = password; // use the generateHash function in our user model
newUserMysql.username = username;
newUserMysql.nickname = nickname;
var insertQuery = "INSERT INTO penguins (igloos, igloo, moderator, inventory, email, password, username, nickname ) VALUES ('1','1','" + moderator + "','" + inventory + "','" + email + "', + UPPER(MD5('" + password + "')), '" + username + "', '" + username + "')"; + connection.escape(username); + connection.escape(password); + connection.escape(nickname); + connection.escape(email);
connection.escape(nickname);
connection.escape(inventory);
connection.escape(igloo);
connection.escape(igloos);
connection.escape(moderator);
connection.escape(id);
connection.escape(username);
connection.escape(email);
connection.escape(nickname);
connection.escape(password);
username.toString();
password.toString();
igloos.toString();
igloo.toString();
moderator.toString();
inventory.toString();
email.toString();
nickname.toString();
id.toString();
console.log(insertQuery);
connection.query(insertQuery, function(err, rows) {
newUserMysql.id = rows.insertId;
return done(null, newUserMysql);
});
}
});
}));
// =========================================================================
// LOCAL LOGIN =============================================================
// =========================================================================
// we are using named strategies since we have one for login and one for signup
// by default, if there was no name, it would just be called 'local'
passport.use('local-login', new LocalStrategy({
// by default, local strategy uses username and password, we will override with email
usernameField: 'email',
passwordField: 'password',
passReqToCallback: true // allows us to pass back the entire request to the callback
},
function(req, email, password, username, nickname, done) { // callback with email and password from our form
connection.query('SELECT * FROM penguins WHERE username = ?' + username, function(err, rows) {
username.toString();
connection.escape(username);
if (err) return done(err);
if (!rows.length) {
return done(null, false, req.flash('loginMessage', 'No user found.')); // req.flash is the way to set flashdata using connect-flash
}
// if the user is found but the password is wrong
if (!(rows[0].password == password)) return done(null, false, req.flash('loginMessage', 'Oops! Wrong password.')); // create the loginMessage and save it to session as flashdata
// all is well, return successful user
return done(null, rows[0]);
});
}));
}
You're mixing string concatenation with placeholders. In short:
Never use something like this but it's technically valid even though it's vulnerable:
connection.query('SELECT * FROM penguins WHERE username = ' + username, ...
Always use something like this which is valid and not vulnerable:
connection.query('SELECT * FROM penguins WHERE username = ?', username, ...
Mixing both like this will create a syntax error:
connection.query('SELECT * FROM penguins WHERE username = ?' + username, ...
because the 'SELECT * FROM penguins WHERE username = ?' string will get concatenated with the value of username and will likely produce something like:
'SELECT * FROM penguins WHERE username = ?johndoe'
which is not valid SQL. Escaping the string will not help here.
See this answer for more details about character escaping and placeholders: