What are good ways to prevent SQL injection?

Question

I have to program an application management system for my OJT company. The front end will be done in C# and the back end in SQL.

Now I have never done a project of this scope before; in school we had only basic lessons about SQL. Somehow our teacher completely failed to discuss SQL injections, something which I have only now come in contact with by reading about it on the net.

So anyway my question is: how do you prevent SQL injections in C#? I vaguely think that it can be done by properly masking the text fields of the application so that it only accepts input in a specified format. For example: an e-mail textbox should be of the format "[email protected]". Would this approach be sufficient? Or does .NET have pre-defined methods that handle stuff like this? Can I apply a filter to a textbox so it only accepts email-address format or a name textbox so it doesn't accept special chars?

Solution 1

By using the SqlCommand and its child collection of parameters all the pain of checking for sql injection is taken away from you and will be handled by these classes.

Here is an example, taken from one of the articles above:

private static void UpdateDemographics(Int32 customerID,
    string demoXml, string connectionString)
{
    // Update the demographics for a store, which is stored  
    // in an xml column.  
    string commandText = "UPDATE Sales.Store SET Demographics = @demographics "
        + "WHERE CustomerID = @ID;";

    using (SqlConnection connection = new SqlConnection(connectionString))
    {
        SqlCommand command = new SqlCommand(commandText, connection);
        command.Parameters.Add("@ID", SqlDbType.Int);
        command.Parameters["@ID"].Value = customerID;

        // Use AddWithValue to assign Demographics. 
        // SQL Server will implicitly convert strings into XML.
        command.Parameters.AddWithValue("@demographics", demoXml);

        try
        {
            connection.Open();
            Int32 rowsAffected = command.ExecuteNonQuery();
            Console.WriteLine("RowsAffected: {0}", rowsAffected);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }
}

Solution 2

My answer is quite easy:

Use Entity Framework for communication between C# and your SQL database. That will make parameterized SQL strings that isn't vulnerable to SQL injection.

As a bonus, it's very easy to work with as well.

Solution 3

SQL injection can be a tricky problem but there are ways around it. Your risk is reduced your risk simply by using an ORM like Linq2Entities, Linq2SQL, NHibrenate. However you can have SQL injection problems even with them.

The main thing with SQL injection is user controlled input (as is with XSS). In the most simple example if you have a login form (I hope you never have one that just does this) that takes a username and password.

SELECT * FROM Users WHERE Username = '" + username + "' AND password = '" + password + "'"

If a user were to input the following for the username Admin' -- the SQL Statement would look like this when executing against the database.

SELECT * FROM Users WHERE Username = 'Admin' --' AND password = ''

In this simple case using a paramaterized query (which is what an ORM does) would remove your risk. You also have a the issue of a lesser known SQL injection attack vector and that's with stored procedures. In this case even if you use a paramaterized query or an ORM you would still have a SQL injection problem. Stored procedures can contain execute commands, and those commands themselves may be suceptable to SQL injection attacks.

CREATE PROCEDURE SP_GetLogin @username varchar(100), @password varchar(100) AS
DECLARE @sql nvarchar(4000)
SELECT @sql = ' SELECT * FROM users' +
              ' FROM Product Where username = ''' + @username + ''' AND password = '''+@password+''''

EXECUTE sp_executesql @sql

So this example would have the same SQL injection problem as the previous one even if you use paramaterized queries or an ORM. And although the example seems silly you'd be surprised as to how often something like this is written.

My recommendations would be to use an ORM to immediately reduce your chances of having a SQL injection problem, and then learn to spot code and stored procedures which can have the problem and work to fix them. I don't recommend using ADO.NET (SqlClient, SqlCommand etc...) directly unless you have to, not because it's somehow not safe to use it with parameters but because it's that much easier to get lazy and just start writing a SQL query using strings and just ignoring the parameters. ORMS do a great job of forcing you to use parameters because it's just what they do.

Next Visit the OWASP site on SQL injection https://www.owasp.org/index.php/SQL_Injection and use the SQL injection cheat sheet to make sure you can spot and take out any issues that will arise in your code. https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet finally I would say put in place a good code review between you and other developers at your company where you can review each others code for things like SQL injection and XSS. A lot of times programmers miss this stuff because they're trying to rush out some feature and don't spend too much time on reviewing their code.

Solution 4

SQL injection should not be prevented by trying to validate your input; instead, that input should be properly escaped before being passed to the database.

How to escape input totally depends on what technology you are using to interface with the database. In most cases and unless you are writing bare SQL (which you should avoid as hard as you can) it will be taken care of automatically by the framework so you get bulletproof protection for free.

You should explore this question further after you have decided exactly what your interfacing technology will be.

View more solutions

Comments

[LeonidasFett]

sorry but what do you mean by "writing bare sql"?

[Waihon Yew]

@LeonidasFett: Typically you work with the database by writing code like User.Name = "Joe"; User.Save(); and not code like Database.ExecuteQuery("UPDATE users SET name = 'Joe' WHERE id = 1");. The second is writing bare sql.

[LeonidasFett]

ah ok. now i understand :) so basically i should interface with the database through a framework and not sql?

[Basic]

What you're advocating here is an ORM - Like the entity framework. I use it myself and love it but it does have some drawbacks - like being really slow for bulk operations. The alternative is to use parameterised commands - you write the SQL with placeholders for your variables and then pass the variables in to a different method which takes care of merging them into the query for you (in fact, they're passed seperately to the Db server which handles the rest). See here for more info

[Waihon Yew]

@LeonidasFett: As a rule of thumb yes. If you need to write bare SQL in some specific places the framework will provide a way to do that.

[Waihon Yew]

@Basic: "Avoid as hard as you can" does not mean "never do that on pain of death". It's a rule of thumb expressed in a way that targets someone who has admitted absolutely none to very little at best previous experience.

[Basic]

@Jon I wasn't disagreeing with you, merely pointing out that before you go and build a huge system based on the EF, you should be aware that it's not a magic bullet. I certainly think it's an excellent solution for most scenarios where you're not manipulating tens of thousands of records.

[Basic]

Im not aware of any SQL injection risks with EF/Similar ORMs as they use parameterised queries internally - can you provide a link? Thanks

[nerdybeardo]

The example I gave above with the stored proc would still be susceptible to a SQL injection attack even if you use an ORM like Entity Framework to call the procedure because the vulnerability is in the proc itself. Therefore what I was trying to convey is that you cannot simply use an ORM and think that you have covered 100% for SQL injection attack cases. Here's a link for more information: troyhunt.com/2012/12/stored-procedures-and-orms-wont-save.ht‌​ml

[Basic]

If you're using an ORM, why would you want to use a sproc? But yeah, I take your point that should you want to do something manual then you could introduce vulnerabilities

[LeonidasFett]

well as of now there are about 3000 records in the exisiting db, but this will be expanded dramatically during the course of this year. i will list the two choices (EF & parameterised commands) with advantages and disadvantages of each and let my project supervisor decide :D

[nerdybeardo]

While I agree that as of late the usage of stored procs has dropped given the rise of the ORM, still there are many business situations which do call for their use. If for no good reason then maybe you have a data architect that requires their use for certain processes or performance reasons. I don't know of many databases which do not have a stored proc in them even today, therefore diminishing their significance in the SDLC could lead to a SQL injection flaw in an application. We developers sometimes live in a world of what we "want" rather than what the business reality is.

[saluce]

This isn't always an option, though, when you don't know what the returning data is going to look like.

[rollsch]

What if I want to actively identify an attempt of sql injection. Are there any good libraries to couple with this approach so I can identify and log an attack?

[Oliver]

The first and simplest approach for SQL injection is the approach to end the current string and statement by starting your value with a single or double quote followed by a brace and semicolon. So by checking the given input if it starts with those would be a good hint, maybe by a regex like ^\s*['"]\s*\)\s*;. That's not the only way, but the most common one.

[Richardissimo]

Note that SqlCommand is also IDisposable so should be in a using block. And you may want to read can we stop using AddWithValue.

[Rez.Net]

I do not like EF. I prefer Light ORMS like Dapper and Insight.Database. When using ORMs you can see/write the exact query that is to execute.

[karezza]

Note, I implemented this code and it crashed upon encountering an 'n' with a '~' above it. Before that happened it was like a miracle, now I'm not sure sure about other languages ... which i was hoping this would make safe.

[Liam]

As a negative it adds a lot of performance overhead