-1

I am writing this code to protect my server from SQL injection. The goal is to insert the [BloCKiT] in front of whatever is matched. Please don't split the word using by space because it will not work with this case. For example "s=290';DECLARE%". This would cause an error.

Please see the comment within the code and thank you.

The code below is written under c#.

string MyOutPut = "";
string PatternAnywhereFromWord = "declare|exec|insert|update|delete|varchar|cast";//search any within the word CASE-INSENTIVE. This is the regular expression
string AttachmeMe = "[BloCKiT]";//Insert this string into the statement

//find patterns case-insensitive anywhere within the statement and attach the AttachmeMe variable in front of the matched position
string InputStatment = "delete s=290';DECLARE%20@S%20NVARCHAR(4000) ;insert into update all xdelete * from database exec";

//some logic here. I plan to write some loop but i think i would perform pretty slow
MyOutPut = "YOUR LOGIC HERE";

//The result should be   [BloCKiT]delete s=290';[BloCKiT]DECLARE%20@S%20NVARCHAR(4000) ;[BloCKiT]insert into [BloCKiT]update all x[BloCKiT]delete * from database [BloCKiT]exec
Dan Abramov
  • 264,556
  • 84
  • 409
  • 511
John Lovus
  • 91
  • 1
  • 9

3 Answers3

7

Are you being serious? Because if you are, don't do this.
Do you think you can always outsmart Little Bobby Tables? Especially with such a naive solution.

drop database master; --oops

Instead, use SQL parameters to make any user input safe.

Community
  • 1
  • 1
Dan Abramov
  • 264,556
  • 84
  • 409
  • 511
  • i do use sql the parameters, i believe it may not be strong enough. see sample below. mySqlCommand.Parameters.Add("@ColumnName", SqlDbType.Text).Value = "delete s=290';DECLARE%20@S%20NVARCHAR(4000)"; – John Lovus Aug 02 '11 at 00:10
  • I don't currently see your point.. Can you do a successful injection with this? – Dan Abramov Aug 02 '11 at 00:12
  • i haven't test yet and do not know how the Parameters.Add would escape the data besides the escaping the single quote – John Lovus Aug 02 '11 at 00:17
  • Parameters are not concatenated into SQL and are passed separately (in the end of the query). Therefore, there is no need to escape them at all. – Dan Abramov Aug 02 '11 at 00:18
  • just to be safe i like to use my technique plus the Parameters.Add, thanks so much for the comment – John Lovus Aug 02 '11 at 00:21
  • @John: your technique does not make it any safer and may in fact invalidate otherwise good queries. (e.g. if StackOverflow used your procedure, you wouldn't be able to post this question because it contains all “forbidden” keywords--yet isn't dangerous) – Dan Abramov Aug 02 '11 at 00:24
  • 1
    @John: You are provided with a safe way to pass values into queries. Use it. Trying to add your own protection like this is just a waste of time. – MRAB Aug 02 '11 at 00:42
1

Microsoft have a guidelines page for how to avoid SQL Injection Attacks. You should never have to manually parse your sql strings, or generate sql strings manually. As this is prone to errors and makes your solutions rigid and difficult to maintain.

Justin Shield
  • 2,390
  • 16
  • 12
  • i don't want to constrain user input – John Lovus Aug 02 '11 at 00:15
  • @John there's more to it in the document. – Dan Abramov Aug 02 '11 at 00:22
  • 2
    @John: what do you mean you don't *feel* safe? Obviously you can't prevent a hacker from finding your datacenter, breaking into the server room with a machinegun and wiping out all databases. Other than that, *if you really follow all of these techniques*, you're pretty safe. – Dan Abramov Aug 02 '11 at 00:38
  • If someone enters in a SQL injection query into your SQL Parameters you will simply get sql text inside your text column in your db which will not execute any commands on the db. – Justin Shield Aug 02 '11 at 00:47
  • i gotta turn on the sql profiler and see what actually sends to the server... thank so much for all of your suggestions guys. – John Lovus Aug 02 '11 at 01:14
0

This will do what you are trying to do:

MyOutPut = Regex.Replace(InputStatment,
           "(?<tok>" + PatternAnywhereFromWord + ")",
           AttachmeMe + "${tok}");

However, as Dan Abramov says, this is really not an effective approach to protecting against SQL injection attacks. Plus, it will most likely trash various kinds of perfectly legitimate input; and it does nothing about quotes and semicolons, which are generally the main things you worry about with SQL injection.

Nate C-K
  • 5,744
  • 2
  • 29
  • 45
  • Nate C-K, i think your code is almost working... it couldn't insert the AttachmeMe in front of DECLARE – John Lovus Aug 02 '11 at 00:24
  • the result for Nate C-K is [BloCKiT]delete s=290';DECLARE%20@S%20NVARCHAR(4000) ;[BloCKiT]insert into [BloCKiT]update all x[BloCKiT]delete * from database [BloCKiT]exec – John Lovus Aug 02 '11 at 00:25
  • 1
    @John: If this really makes you happy, pass `RegexOptions.IgnoreCase` as the last parameter to `Regex.Replace`. But this doesn't help SQL injection protection in the slightest. Instead, it complicates the code and invalidates possibly good input. Good luck with that. – Dan Abramov Aug 02 '11 at 00:29
  • yep, you are right, that that missed the RegexOptions.IgnoreCase but this is not part of the missing solution. The problem is the regular expression couldn't insert the in front of the ';DECLARE it inserted way before it. – John Lovus Aug 02 '11 at 00:38
  • i gotta turn on the sql profiler and see what actually sends to the server... thank so much for all of your suggestions guys. – John Lovus Aug 02 '11 at 01:14