Finding SQL Injection Attacks with 'ack'

Ovid on 2007-08-20T13:11:57

We are evaluating a third party application. I just ran the following command:

$ ack -i '\b(?:select|update|insert|delete)\b.*\$'  \
   --php include |wc -l
307

A cursory scan indicates that many of those results are, in fact, very dodgy SQL embedding variables directly in SQL rather than using placeholders. Hmm, how many files is that?

$ ack -il '\b(?:select|update|insert|delete)\b.*\$'  \
   --php include |wc -l
92

Needless to say, I don't feel terribly comfortable with this application, but I love ack. This isn't the final nail in the coffin for this application as it's possible that all of these variables are well-sanitized, by for crying out loud, use your frickin' placeholders in SQL! Of course, with 92 potentially vulnerable files, trying to verify that everything is safe seems more trouble than it's worth.

Update: we're not going to use this software. Using this as a starting point, we started digging into the code. One of the final nails was this bit of code (munged to hide the identify of the folks we'll be contacting):

function createInsertForSQL($columns) {
    $values = '';

    foreach ( $columns as $column ) {
        if ( $values != '' ) {
            $values .= ', ';
        }
        $values .= $column;   
    }
    return $values;
}

I see at least two bugs there, both of which could be very serious.