Protect Your Database From SQL Injection
Mark Kruger aka ColdFusion Muse, wrote a couple of good articles regarding SQL injection and shows a few examples of how you can be attacked. I fully agree with his recommendation to always, always, always, use CFQUERYPARAM. I also take it another step further (which he also recommends) by validating the input before it's even passed to the SQL code itself.
For example, if your using a URL.id variable to check for a detailed record, assuming your using a .CFC to store your query and then using CFINVOKE on the details page/template, I would do something like:
- Wrap the WHERE clause in my SQL SELECT statement with a CFIF statement
- And, use CFQUERYPARAM
- Then, on the details page, use another CFIF statement to validate the URL (or form, etc.) being input
Code in CFC:
<cffunction name="getNews"
displayname="Get record details from db"
access="public"
returntype="query"
output="no">
<cfargument name="id"
type="numeric"
required="no"
default="1" />
<cfset var returnNews = "" />
<cfquery name="returnNews" datasource="#application.dsn#">
SELECT tblNews.NewsID
, tblNews.NewsTitle
FROM tblNews
WHERE 0 = <cfqueryparam value="0" cfsqltype="cf_sql_numeric" />
<cfif isDefined("ARGUMENTS.id") and isNumeric(ARGUMENTS.id)>
AND tblNews.NewsID = <cfqueryparam value="#ARGUMENTS.id#" cfsqltype="cf_sql_numeric" />
</cfif>
</cfquery>
<cfreturn returnNews />
</cffunction>
Using this code on the details page would help to prevent the component from even being invoked if being attacked:
<cfif isDefined("URL.id") AND isNumeric(URL.id)>
<cfinvoke component="extensions.components.newsqueries"
method="getNews"
returnvariable="returnNews">
<cfinvokeargument name="id" value="#URL.id#" />
</cfinvoke>
</cfif>
Using this code on the details page (option 2) could fire the component but not pass in the argument:
<cfinvoke component="extensions.components.newsqueries"
method="getNews"
returnvariable="returnNews">
<cfif isDefined("URL.id") AND isNumeric(URL.id)>
<cfinvokeargument name="id" value="#URL.id#" />
</cfif>
</cfinvoke>
</cfif>
Then, you could simply display some sort of friendly error message if no record is returned.
By using these methods, if someone were to play around with the the URL.id by entering 'http://www.yourdomain.com/details/?id=1;DROP%20DATABASE%20foo' (you get the idea), then the variable wouldn't even be passed in the first place.
In addition to protecting your SQL, I highly recommend testing for vulnerabilities too. There are a number of ways to do this up to and including:
- manually typing in code in your URL's to try and inject SQL into your own site
- paying a third-party to do testing for you
- using specially-designed software
Sometimes I get leary of directing people to any 'automated' solutions because it can lead some to a false sense of security by relying on the software to catch everything. That being said, the smart people at HP have been putting their big brains to work trying to come up with a way to address the issue of SQL Injections - spurred on by our friends at Microsoft.
The result is a product called 'Scrawlr'. Scrawlr is short for SQL Injector and Crawler. It will crawl your website simultaneously analyzing the parameters of each page for vulnerabilities. They have a pretty decent white paper at http://products.spidynamics.com/asclabs/sql_injection.pdf. You can download Scrawlr at https://download.spidynamics.com/Products/scrawlr/. I've personally used it and was surprised at how quickly the software traverses the site exploring for 'injection opportunities.' I purposely created a page where I did not properly 'protect' my SQL and sure enough, Scrawlr found it.
Once again, do not only rely on software to detect your vulnerabilities. Used properly, this could be one more tool in your developer tool box to help combat those evil SQL Injectors.
More information about scrawlr can be found at http://www.communities.hp.com/securitysoftware/blogs/spilabs/
archive/2008/06/23/finding-sql-injection-with-scrawlr.aspx?jumpid=reg_R1002_USEN
Comments
Awesme article and thanks for the props. I like your practicle examples too.
In your example where you check "isNumeric( )" on the url.id - I have always liked :
<cfif val(url.id)>
... code here
</cfif>
As long as my use case dictates that the url param should always be an int I'm golden.. cuts down on coding - but I suppose it is less semantic. I've used that word semantic 3 times today :)
val(1234) = 1234
val(123abc) = 123
val(123abc213) = 123
val(abcd) = 0
The reason for the CFIF statements was to show a preference to at least test what's being sent from the user whether it be from the url, a form, etc. Also, since the query's ID argument is not required, the query should return all records if the argument is not supplied, making the query a bit more flexible and reusable.
On the details page one could always add something like: <cfif returnNews.recordcount NEQ 1>show friendly error message here.<cfelse>output query here.</cfif>
i found this patch on the microsoft website:
"MS08-040: Description of the security update for SQL Server 2000 GDR and MSDE 2000: July 8, 2008 http://support.microsoft.com/kb/948110/en-us";
does someone know, if this patch protect the sql server of the injetcion?
thanks for your answers!
Patches alone do NOT protect any database from SQL injection, good programming, testing and monitoring will curb the tide. Whenever you have an application that accepts (even requires) input and the mere use of dynamic variables, etc. you create holes or opportunities for malicious users to flex their exploitation muscles. It's truly up to you to stay on top of these threats by using proven techniques to combat these evil-doers.
However, in your first example, you don't need the cfqueryparam for "WHERE 0 = <cfqueryparam value="0" cfsqltype="cf_sql_numeric" />". Using "WHERE 0 = 0" would be more efficient.
You only need cfqueryparam with variables, if you've got a hard coded value (like in your example), then you're wasting processing power of both CF and the DB by trying to parameterize and bind something that is never going to change. Granted, it's not enough processing power to ever notice, but it's still unnecessary processing on both systems. Plus, a hard coded value like that isn't subject to SQL Injection.
Good points. Guess I was just trying to drive home the point to use CFQueryParam. Thanks!
I couldn't agree with you more. In fact, in a related post (http://www.stephenwithington.com/blog/index.cfm/20...) I do mention (and show examples) of that too. Thanks for bringing that up.