Node.js sql injection parse error

Question

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]);

        });



    }));

}

rsp · Accepted Answer · 2017-07-17 13:07:59Z

1

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:

How to escape mysql special characters with sockets.io/node.js/javascript

answered Jul 17, 2017 at 13:07

rsp

112k31 gold badges210 silver badges185 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Java Sherwood Over a year ago

I did this: connection.query('SELECT * FROM penguins WHERE username = ?', username, function(err, rows) { but I still receive the sql error.

Ben Fortune Over a year ago

Should be [username]

Java Sherwood Over a year ago

@BenFortune after doing that, I receive the following 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 '?' at line 1

Ben Fortune Over a year ago

You did connection.query('SELECT * FROM penguins WHERE username = ?', [username], ... ?

Java Sherwood Over a year ago

Yes I did: connection.query('SELECT * FROM penguins WHERE username = ?', [username], function(err, rows) {

Java Sherwood · Accepted Answer · 2017-07-17 14:15:08Z

0

I found the issue.

Vulnerable line: connection.query('SELECT * FROM penguins WHERE username = ?', username, ...

The last part: username = ?

If someone registers with ? in it, it CREATES the SQL error.

A simple fix: connection.query("select * from penguins where id = "+id,function(err,rows){

Another fix: connection.query("select * from penguins where username = '"+username+"'",function(err,rows){

Cheers!

answered Jul 17, 2017 at 14:15

Java Sherwood

91 silver badge6 bronze badges

Collectives™ on Stack Overflow

Node.js sql injection parse error

2 Answers 2

5 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related