Using Eclipse to find queries that aren't using <cfqueryparam />

Posted by Dan on Jul 23, 2008 @ 11:13 AM

With all the chatter about recently SQL injections attacks, I thought I'd try and whip up a regex I could use in Eclipse/CFEclipse to find <cfquery> tags that have exposed variables (strings wrapped in #...#) that don't use the <cfqueryparam /> tag.

Well I'm far from a regex master, here's what I came up with:

<cfquery\s[^>]*>([^#]*(((?<!value=")#[^#]*#)))((?<!</cfquery)[^>]*?)</cfquery>

The query does not explicitly check for the token <cfqueryparam, but instead checks to make sure that CF variables are preceded with the string value="—which is the attribute used in <cfqueryparam />.

The query isn't perfect and may pick up occasional false positives, but from my testing it seems to work pretty well. If you have some improvements to the regex, make sure to post a comment and I'll update the post with the most recent version.

Categories: HTML/ColdFusion

8 Comments

  • Andy Matthews's Gravatar
    Andy Matthews
    Dan...

    My coworkers and I all installed QPScanner and it works excellent. One thing that we noticed that threw false positives all the time are variables inside cfquery blocks which can't be queryparamed. Those include dynamic TOP clases, or dynamic WHERE clauses, group by, etc.

    It would be awesome to try and account for those.
  • Dan G. Switzer, II's Gravatar
    Dan G. Switzer, II
    I don't know that I'd consider those "false" positive, since you need to make sure that you have something in place for those variables to ensure that you aren't vulnerable SQL injection. Whether that's use val() for numeric variables (such as in TOP statement) or limiting strings to specific values (like comparing to make sure the value is in a list for ORDER BY or GROUP BY statements.)

    At some point a scanner, without building a lot of additional rule checking that you'd have to configure, it's just not going to be able to avoid every exception. And I rather have something that's being conservative and showing everything, than possibly missing out on something.
  • Bob Silverberg's Gravatar
    Bob Silverberg
    That's a great idea, Dan. I like that 'cause I'm pretty much always in Eclipse anyway. I just ran it on a project and came up with a few queries that were QofQ's. Knowing very little about regex, is it feasible to change the regex to excude cfquery tags that have a dbtype attribute of "query"?
  • Peter Boughton's Gravatar
    Peter Boughton
    As Dan said, detecting everything and only excluding known safe values is very much a deliberate choice - making it too smart means potentially missing valid risks, and I'd rather give false positives than do that.

    Having said that, if anyone does have specific false positive examples, let me know and I'll see what I can do - just send them to qpscanner_project~hybridchill.com

    The next release will be accompanied by an Eclipse plugin.
    I'm also planning to allow specifying of output format, so it can be automated with Ant.
    (If anyone has any other feature requests, feel free to send them to the email address above.)
  • Andy Matthews's Gravatar
    Andy Matthews
    Peter...

    Two things that I think would be good additions to QPScanner:

    1) The ability to set a timeout for the scan. Our codebase has hundreds of directories, and about 3000+ files. The scan timed out before even completing our admin directory.

    2) The ability to exclude directories. There's a few directories that were completely fixed but had a few queries that couldn't be paramed due to dynamic ORDER BY, or other things like that. It would be nice if we could specifiy directory name exclusions. I was able to modify the code do that that, but I had to take it in and out when we needed to scan that directory again.
  • Peter Boughton's Gravatar
    Peter Boughton
    Thanks Andy, both good suggestions, will add them to next release. :)
  • Aaron Longnion's Gravatar
    Aaron Longnion
    Another RegEx - http://cfzen.instantspot.com/blog/2008/07/16/RegEx...

    btw - your blogCFC version doesn't allow plus signs in emails - http://cfzen.instantspot.com/blog/2008/04/30/Does-...
  • Dan G. Switzer, II's Gravatar
    Dan G. Switzer, II
    @Aaron:

    I thought I updated the isEmail check already, but I just updated the code so it should be working.

    Also, I'm curious as to how my regex compares to yours. In your case, you don't actually check the string between the <cfquery> tags, where as my version specifically examines the text between that tag. It should also work with any kind of query (select/insert/update/etc)

Comments for this entry have been disabled.