Fixing SQL injection forms in a big asp.net C# web application

Question

I have to fix a project that is vulnerable to SQL injection.

All the forms in every page on the project do not use parametrized query but simply string query.

For example I have the search page, and looking at the code behind I see that there is a method CreateQuery() that creates the query basing on the text fields as example:

string sQuery = "";
sQuery += "b.name like '%" + txtName.Text + "%'";

Then in the btnSearch_Click() I have the method that does the query:

query = CreateQuery();
var totalList = GetAllBlaBla(query);

My question is:

Since I have hundreds of forms and thousands of formText and values to FIX, is there a "quick" solution to implement like

1. A global function that parametrizes the query or handle the situation in some way?
2. Since in every class the query is executed in the SubmitButton_Click() code behind method, can I handle the situation here, of course in every class?
3. Should I modify every form and every entry in the form codebehind to parametrize the SQL string, that is gonna take one million of years?

4. (Edit) What about Encode/Decode input values? SO that the example above will be:

   string sQuery = "";
   var txt = var txt = HttpUtility.HtmlEncode(txtName.Text);
   sQuery += "b.name like '%" + txt + "%'";
   

   Is this a possible temporary patch?

5- (Edit) Is this a possible solution, or it simply does not change anything?

        cmd.Parameters.Add("@txtNameParameter", SqlDbType.VarChar);
        cmd.Parameters["@txtNameParameter"].Value = txtName.Text;
        sQuery += "b.name like '%" + (string)cmd.Parameters["@txtNameParameter"].Value + "%'";

The problem is that I have to return a string because the logic that handles the query is defined in another business class that takes a string as a query, I cannot give it a CommandType or SqlDataAdapter...

Suggestion?

Thanks in advance.

Answer 1

You already know there is a problem; IMO, any "quick" fix here is likely to reduce the attack surface, but is not likely to prevent determined abuse; simply, blacklisting is truly hard and there are some really bizarre inputs readily available on black-hat (and, as samples, on white-hat) sites. These are not always easily recognizable as abusive. It isn't all ' drop table Customers -- ;p

WHATEVER you do, I would advise doing it properly; parameters. Tools like dapper might reduce the code you need, though:

sQuery += "b.name like '%'+@text+'%'"
...
conn.Execute(sQuery, new {text=txtName.Text});

(which is easier than handling all the parameters etc manually)

Answer 2

1. Reduce the permissions of the database account that your application uses to access data. Hopefully it doesn't have sysadmin. Remove permissions to drop tables. If the account is used only for data retrieval, remove all permissions to update data. You might even consider setting up views, locking them down, and using them instead of direct table access.

2. Turn on ASP.NET Request Validation, described here. This automatically checks all traffic for malicious character sequences.

3. If possible, consider adding an event handler to Global for OnBeginRequest that inspects the incoming data and performs white list checks on all inputs. Not sure how well this maps to your problem but the nice thing is you only have to do it in once place and it will affect the whole site.

4. Ensure that all of your pages call Page.Validate to ensure that the client-side validation is also enforced on the server side.

5. Begin the long hard work of adding field-specific white list validation to every control, and figure out a long term plan to move onto parameterized database calls.

Answer 3

Modify every query + validate every input.

Takes time but think about the guy who will maintain and add features to that "big web application" (it may be you).

Answer 4

<customErrors mode="On"/> 

This will prevent users to see the errors so a potential hacker would have very few clues how to exploit this security door, based on the error messages he/shes sees
Add Elmah to log errors.
Rewrite every query to use Parameters or use an ORM.
Any javascript based solution is useless, since a "hacker" for sure knows how to disable it.

Answer 5

The Plain Way

Estimate the number of replacements by searching the project for string sQuery = ". Multiply it by the time you plan spending on fixing a single query (e.g. one fix in 5 minutes, plus a coffee break each 10 fixes).

Add time for testing the whole website.

Then tell management the fix is going to be huge, give them the estimate and just do it.
Surely you'll have headaches for a couple of days but at least you'll have the thing working.

The Creative Way

If you literally mean hundreds and thousands of such forms with nearly identical code (e.g. query always created in CreateQuery, executed in SubmitButton_Click), I would consider learning to use Visual Studio regular expression syntax and crafting a couple of very accurate search & replace patterns.

This saved me hours of work in one project, but you'll need to be really precise with regexps and make sure you understand what you're doing.

Another option, again, when you're sure it is worth it, is to write a tool that will rewrite C# sources.
If all you need is a simple transform like the one Marc mentioned, it could take a couple hours of work.
But you can fail miserably here so it's a risky route.

Answer 6

find all textbox controls on each page during pageload and disable special keys press handling