5

I initially discovered that this was an issue when I tried to search for terms that had been prepended with a hashtag, which it turns out is a comment delimiter in SQL. The search returned nothing, because it ignored the #term that came after the hashtag.

So now I'm having trouble finding the proper way of escaping the user's input. It seems to me that this would both solve the hashtag issue and also address the much larger problem, SQL injection.

Here is the snippet I am working with specifically:

function (term) {
  term = term.toLowerCase()
  return db('ticket')
    .select('*')
    .where(db.raw('lower(question)'), 'like', `%${term}%`)
    .orWhere(db.raw('lower(note)'), 'like', `%${term}%`)
    .orWhere(db.raw('lower(user_name)'), 'like', `%${term}%`)
}

I did find this and this SO article that seemed close, as well as a couple other things. Also, Knex's docs and other sources recommend parameterized binding as a method to safeguard against SQL injection.

I'm just having trouble finding a clear example that can be explained to me in JavaScript or using Knex.

Community
  • 1
  • 1
Mike Fleming
  • 2,593
  • 4
  • 14
  • 24
  • Is your DB Collation specifically case-sensitive? You don't need to use `LOWER` with the `LIKE` operator - this also means your query will run very slowly because it can't use indexes. – Dai Nov 19 '16 at 01:43
  • IMO. "SQL Injection" is something like "Problem 2000": a lot of noise, but nothing seriously. There are two simple rules to avoid any "SQL injections": 1) Use parametrized queries at the front/middle end and 2) Setup the appropriate DB level security on the DB objects. – Abelisto Nov 19 '16 at 02:03
  • Thanks for the response. @Dai I tried your suggestion but it appears to be case sensitive. I'm relatively new to db land, could you recommend some resources to learn about using indexes and collation that you mentioned? Never heard the terms before. – Mike Fleming Nov 19 '16 at 15:33
  • @Abelisto I'm not terribly worried about this as the app is an internal tool, but it seems like good practice to protect from. You're suggesting that I sanitize my user inputs upstream before sending them to my backend? Thanks! – Mike Fleming Nov 19 '16 at 15:34

2 Answers2

9

I'm not a Knex.js user, but looking at the docs it seems that Knex's use of JavaScript object syntax to define predicates is how it achieves parameterization.

However as you're using built-in functions you need to use whereRaw.

Looking at the docs ( http://knexjs.org/#Builder-whereRaw ) and ( http://knexjs.org/#Raw-Bindings ) I think you want to do this:

.whereRaw('question LIKE :term OR note LIKE :term OR user_name LIKE :term', { term: '%' + term + '%' ] } )

Knex doesn't have an orWhereRaw, so you should use the longhand version if you want to logically separate the predicates:

term = '%' + term + '%';

.orWhere( knex.raw( 'question  LIKE ?', [ term ] ) )
.orWhere( knex.raw( 'note      LIKE ?', [ term ] ) )
.orWhere( knex.raw( 'user_name LIKE ?', [ term ] ) )

Note ? is for positional parameters, and :term is for named parameters.

Dai
  • 141,631
  • 28
  • 261
  • 374
  • Thanks for the response. Yeah, both of those options that you presented work perfectly. Even so I feel like I am missing the point of where they are fundamentally different than what I was already doing. I had a template string decorated with % inside my orWhere. Does setting the term as an object in the first example or an array in the second achieve parameterized binding? edit: should also note that I still cannot retrieve hashtags from the db, although I can insert them. – Mike Fleming Nov 19 '16 at 15:27
3

It seems that the only time in which you really need to worry about sql injection is if you are using knex.raw() or any other pure sql command. In other words, Knex escapes the input for you automatically.

As for the hashtag issue, after messing around with PG Commander I discovered that I could search for #'s just fine. I just needed to url encode hashtags before sending them to my backend... A little embarrassing but I learned something new today.

Mike Fleming
  • 2,593
  • 4
  • 14
  • 24