2

There has been some discussion on this topic (e.g. Preventing SQL injection in Node.js )but really no clear-cut clarity or a deep discussion, let alone good documentation anywhere. The node-mysql docs discuss prevention of SQL injection and some escape functions. However, it is unclear how these functions prevent SQL injection. The manual says "Strings are safely escaped." Nothing more... Is that limited to escaping some characters only?

There seem to be other equivalents in node-mysql for the same function as in connection.escape and pool.escape with an emphasis again that these functions are used to prevent SQL injection.

There also does not seem to be support for a true prepare statement in node-mysql. The plans and documentation are again unclear on this. Node-mysql is clearly a very popular module in the node.js environment and fairly stable at least in the limited experience I had with it. What are the Best Practices for preventing SQL injection in node-mysql?

Community
  • 1
  • 1
Sunny
  • 9,245
  • 10
  • 49
  • 79
  • 1
    I'm certainly no expert in using node-mysql or escaping sql, but, i would suggest using the parameterized query syntax any time you need to create the sql query yourself. This module does seem to support it. If you're using any of the other methods, such as find, findById, clone, etc, you don't have to worry about it because the query that is built is built using the parameterized query syntax. – Kevin B Sep 21 '15 at 16:06

2 Answers2

3

Remember that SQL injections are caused by hostile strings being interpreted as commands, not by blocking commands. Are you sure that you're getting the original string back, not a stringified version?

For example there's a huge difference between these two: "test" and "'test'".

Generally only harmful characters are escaped, the rest are left as-is.

Using the low-level driver is best avoided. Try and use a library like Sequelize to provide some abstraction and more support. That module supports placeholder statements that generally make escaping a non-issue, it's handled automatically.

See the section on raw queries with replacements where you have the ability to do this:

sequelize.query('SELECT * FROM projects WHERE status = ?',
  { replacements: ['active'], type: sequelize.QueryTypes.SELECT }
).then(function(projects) {
  console.log(projects)
})

There's no risk of user data leaking through because you've supplied it as an explicit value that's handled properly, not an inline string in the query.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • 1
    the [mysql](https://www.npmjs.com/package/mysql) module (of which node-mysql depends on) also has support for parameterized queries. – Kevin B Sep 21 '15 at 16:02
  • 1
    @kevinb If it does, that's something to make use of. Parameterized queries are a lot harder to get wrong, and injection bugs, if they occur, are usually the result of multiple mistakes, not just one. – tadman Sep 21 '15 at 16:03
  • I think using sequelize to just get over one issue may not justify its usage. As for using parameterized queries, is that not where SQL injection comes into play in the first place - When assigning user-supplied values to these parameters? I think that using true prepare statements to use parameterized queries does go a long way in preventing SQL injection but such prepared statements (as in PHP) are not supported here. Or are they in some form? – Sunny Sep 21 '15 at 16:17
  • 1
    @Samir The #1 way to prevent SQL injection is to be extremely disciplined about using prepared statements. As Kevin B points out the MySQL module does support these, so make use of them. SQL injection is simple: You fail to escape something properly and it ends up in your query. The most common way this happens is sloppy query composition. Parameterized queries side-step that completely, you never put user data in a potentially harmful spot in the first place. As for Sequelize, it's a great library that helps on many levels, not just placeholders. – tadman Sep 21 '15 at 16:20
  • @tadman, certainly agree with you on the "escape" part. I want to understand the precise scope or useful examples of the "escape" function as provided by node-mysql to be used in parameterized queries. It does escape quotes and some such characters that can syntactically mess up the query but what more does it do to qualify as a SQL-injection avoidance? – Sunny Sep 21 '15 at 16:38
  • 1
    @Samir That's all it has to do. So long as your database correctly interprets the string as a value and not as a statement your escaping function has done its job. This is no different than what you have to do in HTML to avoid user content altering the structure of the page. – tadman Sep 21 '15 at 16:47
  • 1
    I'm surprised no one has mentioned prepared statements/bind parameters. node-mysql doesn't use them, so there's an inherent security risk in using user-supplied values in queries. However, if you use prepared statements in node-mysql2, then you're safe from SQL injection, even if you use user-supplied values. Sequelize uses node-mysql as it's mysql driver, so it's vulnerable. – Cully Mar 05 '16 at 07:58
  • @Cully Sequelize uses prepared statements to avoid this issue, but if you go out of your way you can create injection bugs. There's no issue with `node-mysql` if you're using Sequelize properly. – tadman Mar 06 '16 at 23:29
  • @tadman Sequelize doesn't use prepared statements. It simulates them in code. They basically just "escape" values. Real prepared statements are handled by the database itself. – Cully Mar 06 '16 at 23:46
  • @Cully You're just arguing implementation details. Emulating them is perfectly acceptable if you're just concerned about data integrity. – tadman Mar 07 '16 at 01:46
3

still sequelize is vulnerable 

db.query('SELECT Desc FROM Items WHERE Username IN (:names)', {
  replacements: {
    names: ["Bobby", "'); DELETE FROM Items WHERE 1=1; --')"]
  }
});
Rizwan Patel
  • 538
  • 2
  • 9
  • 27