hiddentao / squel

:office: SQL query string builder for Javascript
https://hiddentao.github.io/squel
MIT License
1.57k stars 231 forks source link

string escape is incorrect, especially when string contains \n or ', which may cause SQL injection #366

Open yibn2008 opened 6 years ago

yibn2008 commented 6 years ago

IMPORTANT: This issue may cause SQL injection:

const unsafeString = "' AND '1' = '1"
console.log(squel.select().from('some_table').where('id = ?', unsafeString).toString())
// output: SELECT * FROM some_table WHERE (id = '' AND '1' = '1')

Problem

for example:

const squel = require('squel')
const lines = 'ABC\nDEF\'IJK'
const json = JSON.stringify({lines: lines})
console.log(squel.select().from('some_table').where('lines = ? OR json = ?', lines, json).toString())

expect:

SELECT * FROM some_table WHERE (lines = 'ABC\nDEF\'IJK' OR json = '{\"lines\":\"ABC\\nDEF\'IJK\"}')

actual:

SELECT * FROM some_table WHERE (lines = 'ABC
DEF'IJK' OR json = '{"lines":"ABC\nDEF'IJK"}')

Workaround

Use sqlstring will escape string value correctly:

const sqlstring = require('sqlstring')
const squel = require('squel')
squel.registerValueHandler('string', (v) => {
  return {
    value: sqlstring.escape(v),
    rawNesting: true
  }
});
demurgos commented 5 years ago

I just tracked down a bug in my code base that was caused by the similar issue: by default, STRINGS ARE NOT ESCAPED!!!

console.log(squel.update()
        .table("t")
        .set("name", "foo'bar")
        .toString())
// Prints:
// UPDATE t SET name = 'foo'bar'

Note that the string is not escaped.

This is an extremely surprising and dangerous default!