1
  1. Is MATCH from MySQL also vulnerable to injection attack? For example:

    """SELECT * 
         FROM myTable 
         WHERE MATCH(myColumnName) AGAINST(%s) 
         ORDER BY id 
         LIMIT 20""" % query
    

    seems to allow arbitrary strings, which looks bad.

  2. If so, I've instead tried - following examples in the Python docs -

    t = (query,)
    statement = """SELECT * 
                     FROM myTable 
                     WHERE MATCH(myColumnName) AGAINST(?) 
                     ORDER BY id 
                     LIMIT 20"""
    cursor.execute(statement, t)
    

    but nothing is returned - even when the string query returned hits in (1) above. Why is that?

  3. In 2), using the placeholder %s instead of ? returns results. Why is this safer than 1) (if at all)? E.g. with the query string I can always close off a string and parenthesis with query=')...' and continue query=') OR otherColumnName LIKE '%hello%' --.

Therefore, is it enough to strip query strings of everything but roman characters or numerals?

outis
  • 75,655
  • 22
  • 151
  • 221
Ken
  • 30,811
  • 34
  • 116
  • 155
  • Using prepared query like above - with `?` makes sure the strings are treated as quoted/escaped when needed. – viraptor Aug 31 '11 at 09:44
  • The [sample](http://sscce.org/) isn't complete. In 2., what's the value of `query`? What's a row you expect it to match? Note that while it doesn't matter in samples, don't use [`SELECT *`](http://stackoverflow.com/questions/321299/what-is-the-reason-not-to-use-select) in production code; select only the columns you need. – outis Aug 31 '11 at 10:08
  • As it happens, I need all of the columns in the table. I could spell this out equivalently by naming all of the columns. – Ken Aug 31 '11 at 23:10
  • Do you need all the columns that may be in the table now and in the future, or only the columns when the SQL statement was written? What happens if you have to add columns? Take a closer look at the question regarding `SELECT *` linked to in my previous comment. – outis Sep 01 '11 at 11:08
  • I only need the columns that are present today, I may or may not need future ones. Therefore I went with replacing '*' by the individual column names. – Ken Sep 01 '11 at 12:33

4 Answers4

4

It doesn't much matter what operators, functions, clauses or any other host-language terms you're using when it comes to injection. Injection is a matter of mixing data and language statements, which happens when you interpolate data into a statement. Prepared statement parameters keep data and statements separate, so they're not vulnerable to injection.

As for ? versus %s for parameters, the MySQLdb documentation for Cursor.execute says the following:

    execute(self, query, args=None)

[...]

Note: If args is a sequence, then %s must be used as the parameter placeholder in the query. If a mapping is used, %(key)s must be used as the placeholder.

outis
  • 75,655
  • 22
  • 151
  • 221
1

When using a parameterized statement with arguments, MySQLdb escapes the arguments and uses Python string formating to re-insert those arguments into the parametrized statement. A single string statement is then sent to the server. You are relying on MySQLdb's ability to properly escape arguments to protect against SQL injection:

MySQLdb/cursors.py:

def execute(self, query, args=None):
    ...
    if args is not None:
        query = query % db.literal(args)

In contrast, oursql sends queries and data to the server completely separately. It does not rely on escaping the data. This should be even safer.

unutbu
  • 842,883
  • 184
  • 1,785
  • 1,677
1

The first method is not correct since it uses basic python string formatting, that will not escape the query string.

The second method is the preferred method of sending a simple query to the server and it will properly escape the query.

You just have a simple bug. You need to replace AGAINST(?) with AGAINST(%s)

The information is also found in the python DbApi FAQ

Mihai Stan
  • 1,052
  • 6
  • 7
0

For my purposes it's enough to sanitise the query string - strip if of all non-alphanumeric characters (in particular ' and )).

Ken
  • 30,811
  • 34
  • 116
  • 155