4

I'm currently reviewing someone's code, and I ran into the following Python line:

db.query('''SELECT foo FROM bar WHERE id = %r''' % id)

This goes against my common sense, because I would usually opt-in to use prepared statements, or at the very least use the database system's native string escaping function.

However, I am still curious how this could be exploited, given that:

  1. The 'id' value is a string or number that's provided by an end-user/pentester
  2. This is MySQL
  3. The connection is explicitly set to use UTF8.
Evert
  • 93,428
  • 18
  • 118
  • 189
  • 1. How? 2. What difference does that make? 3. See point 2. – roganjosh Nov 05 '19 at 21:16
  • 1
    1. 'id' is supplied by a user. 2) different SQL servers have different escaping mechanisms. 3) MySQL escapes differently depending on the character set. – Evert Nov 05 '19 at 21:17
  • Well, if id is supplied by a user then I'd argue that this is the _opposite_ of you, the programmer, having control over the value. MySQL escaping can't do much here because you're using the `repr` string formatting parameter so you're just supplying a string to be executed – roganjosh Nov 05 '19 at 21:18
  • @roganjosh, sorry that's poor phrasing on my part. I'm updating the question to be more explicit. – Evert Nov 05 '19 at 21:19
  • 1
    The best you can do for `id` is check that it is an int. Most vulnerable approaches here don't even bother to do that, so they'd be open – roganjosh Nov 05 '19 at 21:19
  • 1
    @roganjosh, sorry to be super clear. I'm not looking to improve this code. I would use prepared statements. I am looking for proof that this is exploitable. – Evert Nov 05 '19 at 21:20
  • 1
    Sure, I understand what you're asking. Even [Bobby Tables](https://xkcd.com/327/) is enough to demonstrate here :) – roganjosh Nov 05 '19 at 21:21
  • 1
    It wouldn't be, because `%r` will actually escape quotes. – Evert Nov 05 '19 at 21:22
  • Ok. Fair point, I'll have to do some playing – roganjosh Nov 05 '19 at 21:23
  • 1
    @roganjosh thank you! very curious :D – Evert Nov 05 '19 at 21:23

1 Answers1

1

Python drivers for MySQL don't support real prepared statements. They all do some form of string-interpolation. The trick is to get Python to do the string-interpolation with proper escaping.

See a demonstration of doing it unsafely: How do PyMySQL prevent user from sql injection attack?

The conventional solution to simulate parameters is the following:

sql = "SELECT foo FROM bar WHERE id = %s"
cursor.execute(sql, (id,))

See https://dev.mysql.com/doc/connector-python/en/connector-python-api-mysqlcursor-execute.html

The only ways I know to overcome escaping (when it is done correctly) are:

  • Exploit GBK or SJIS or similar character sets, where an escaped quote becomes part of a multi-byte character. By ensuring to set names utf8, you should be safe from this issue.
  • Change the sql_mode to break the escaping, like enable NO_BACKSLASH_ESCAPES or ANSI_QUOTES. You should set sql_mode at the start of your session, similar to how you set names. This will ensure it isn't using a globally changed sql_mode that causes a problem.

See also Is "mysqli_real_escape_string" enough to avoid SQL injection or other SQL attacks?

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • 1
    That doesn't answer the question though; can we inject into `'''SELECT foo FROM bar WHERE id = %r''' % id`? So far, I am failing – roganjosh Nov 05 '19 at 21:46
  • 2
    I appreciate the background, but I'm primarily looking for a way to exploit the above code without making any modifications. My role in this is not an engineer, but I was hired as a 3rd party reviewer. Despite that the source is a red flag, I would like to demonstrate a real issue. – Evert Nov 05 '19 at 21:47
  • It looks like `%r` calls [repr()](https://github.com/python/cpython/blob/2.7/Objects/stringobject.c#L938) which does approximately the same thing as [MySQLConverter](https://github.com/mysql/mysql-connector-python/blob/master/lib/mysql/connector/conversion.py#L114) in the Python connector. Since your character set is UTF8, I don't see either one being vulnerable as long as you use `%r` and not `%s` in your string formatting. – Bill Karwin Nov 05 '19 at 22:02
  • @BillKarwin i guess it really comes down to.. does it do enough? Can it be broken? If your answer is 'definitely not' with proof I would totally accept it. If you're not sure I will keep the question open for a bit longer. – Evert Nov 05 '19 at 22:05
  • @Evert it's very hard to prove that something can't be exploited. I've opened it up to the Python chat room and there is one method that looks like it does the job, but I can't think of any justification of why you'd have it on your backend for now – roganjosh Nov 05 '19 at 22:09
  • Fair! I guess I'm just looking for _some level of certainty_ – Evert Nov 05 '19 at 22:30