Finding SQL Injection Attacks with 'ack'

brian_d_foy on 2007-08-20T13:29:00

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.

---

Yay Perl

petdance on 2007-08-20T14:58:42

There's something to having real honest-to-Larry Perl regexes, isn't there?

You could also use -o to output the regexes that match, if you want, so you could have it spew the offending code.

Ack rocks but...

stu42j on 2007-08-20T15:17:31

I love ack too but I still like the idea of a grep replacement that understands Perl. Are you going to continue with pgrep (or whatever, I like 'grepl')? Or perhaps it could be an optional add-on for ack?

Re:Ack rocks but...

Ovid on 2007-08-20T15:24:42

I will continue working on it. I do like the grepl name. And a cursory Google search show that it appears to be available :)

As for working with ack. you'd have to talk to Andy about that. I've made it pretty extensible, but it's really his call if he wants to find a way to integrate it.

Re:Ack rocks but...

jjore on 2007-08-20T17:49:24

I'm hoping you change pgrep's name to ppgrep or ppigrep because pgrep is already taken by the grep from the "unix tools in perl" project and that's got several years of precedence.

Re:Ack rocks but...

Ovid on 2007-08-20T18:00:25

Hated ppgrep and ppigrep won't make much sense if I ever update this for Perl 6 (which I would love to do, years from now). However, grepl (grep + pl) has been suggested and I love it. It makes perfect sense.