Reflected SQL injection

I’ve been developing a system for a while now and it’s currently in the testing stage, I noticed that a SQL injection vulnerability had crept in. I was quite surprised because my filtering methods are generally quite good and I’ve not seen any SQL errors in the logs for quite some time. Luckly the site wasn’t live so I had plenty of time to see what was going on.

I noticed that all my input was being escaped correctly and I couldn’t understand how the system was producing a SQL error. Then I noticed something, my code was sharing data across 2 tables. So it would query 1 table and then loop through the records and use that data to perform another SQL query on the other table.

This image displays clearly my simple mistake:-

Reflected SQL

So although I was escaping the data originally, I had forgotten to escape it a second time when it was used looping through the data. The SQL data was reflected from table 1 to table 2, a simple code change cleared the problem quite easily but I think it is one of those items that can slip under the radar unnoticed.

9 Responses to “Reflected SQL injection”

  1. Giorgio Maone writes:

    Just another crystal clear case supporting prepared statements / parameters binding.

    Security aside, using a prepared statement should be your first choice in a loop: why would you want to parse the same SQL source over and over again?

  2. Gareth Heyes writes:

    Yep prepared statements would be better I must admit. But the system I had developed wasn’t using them at the time.

    I had to store copies of the same data in separate tables because users can either keep the data as it is or modify it to suit their needs.

    I could explain further but I’d have to give you more details how the system works, what it does etc.

    I just found the vulnerability interesting because my filters didn’t work.

  3. Richard Davey writes:

    I’m curious – you escaped the user input before storing it in Table 1 according to your diagram. How was the data then un-escaped in-between making its way from Table 1 to 2?

  4. Gareth Heyes writes:

    I had a step by step wizard which asked for the required steps and the code worked for data supplied from the user but it didn’t have any escaping code for data interaction. It does now though 😉

  5. Felix Zaslavskiy writes:

    You should escape your input at the level of the query for example a simple enough wrapper around the query function should do. It would work something like this:

    sql_query( “select .. from .. where id=%d”, $id );

    This function would do a mysql_escape_string() call on each parameter before doing a sprintf() on the string.

    So yes you will be doing extra computations because everything will get escaped but you get worthy security.

  6. dancaragea writes:

    There are 6 different situations where you have to escape the data in a web app:
    (GPC means user data from a form)
    1. GPC -> DB (that’s a classic)

    2. GPC -> GPC (when there’s an error processing the form and you present the form back, with the form fields pre-filled with whatever data the user entered the first time)

    3. GPC -> display (when you write back a message like “you entered <blabla>”. This is not the same with gpc->gpc)

    4. DB -> GPC (edit forms)

    5. DB -> display (e.g. to display blog posts, comments, etc)

    6. DB -> DB (rare but you still have to escape this one. I use this case in an application to move messages from a queue table to the inbox table)

  7. ce writes:

    when moving from table to table, why don’t just use
    INSERT INTO … SELECT … FROM …

  8. Gareth Heyes writes:

    It wasn’t as easy as that, the data wasn’t being mirrored but created based on certain conditions.

    The idea was to enable a user to choose a set template of data which can be modified dynamically or overwritten when in the new table.

  9. Gwyn Fisher writes:

    It’s worth noting that prevalent source code analysis tools (such as the one I represent, Klocwork, full disclosure and all that) can find such reflections as well as the more normal scenarios spelled out above. The tools do tend to be pricey, although there are some very capable open source options available to individual developers that are worth checking out.

    Anyway, congratulations on isolating your problem. So many developers let things like this make it all the way through QA and out into the wild, it’s just not funny.

    If you’ve got some time, check out our website for description of the kinds of things we find, or go to wikipedia and check out the prevalent open and commercial tools by looking for “static analysis”.