SQL Injection Is this Good

Question

I have done quite a bit of research on this but I'm still having a problem understanding it. However I want to make sure that I am properly protected. I wrote a function in Classic ASP to help prevent a SQL Injection or possible brute force to the DB. Could you guys give me your own input and suggestions if I need to add to it or remove things or even correct issues to make it more secure? Thank you very much in advance!!

I use this below right before inserting in to a MySQL database.

An example insert:

conn.execute("INSERT INTO " & employees & "(eid, first_name, last_name) VALUES('" & Clng(strEID) & "','" & SQLClean(strfirstname) & "','" & SQLClean(strlastname) & "');")

The function:

Private Function SQLClean(ByVal strString)
    If strString <> "" Then
        strString = Trim(strString)

        'Remove malisous charcters from sql\
        strString = replace(strString,"-shutdown","", 1, -1, 1)
        strString = replace(strString,"\","\\", 1, -1, 1)
        strString = replace(strString,"=","\=", 1, -1, 1)
        strString = replace(strString,",","\,", 1, -1, 1)
        strString = replace(strString,"`","\`", 1, -1, 1)
        strString = replace(strString,"&","\&", 1, -1, 1)
        strString = replace(strString,"/","\/", 1, -1, 1)      
        strString = replace(strString,"[","\[", 1, -1, 1)
        strString = replace(strString,"]","\]", 1, -1, 1)
        strString = replace(strString,"{","\{", 1, -1, 1)
        strString = replace(strString,"}","\}", 1, -1, 1)
        strString = replace(strString,"(","\(", 1, -1, 1)
        strString = replace(strString,")","\)", 1, -1, 1)
        strString = replace(strString,";","\;", 1, -1, 1)
        strString = replace(strString,"+","\+", 1, -1, 1)
        strString = replace(strString,"<","\<", 1, -1, 1)
        strString = replace(strString,">","\>", 1, -1, 1)
        strString = replace(strString,"^","\^", 1, -1, 1)
        strString = replace(strString,"@","\@", 1, -1, 1)
        strString = replace(strString,"$","\$", 1, -1, 1)
        strString = replace(strString,"%","\%", 1, -1, 1)
        strString = replace(strString,"!","\!", 1, -1, 1)
        strString = replace(strString,"*","\*", 1, -1, 1)
        strString = replace(strString,"~","\~", 1, -1, 1)
        strString = replace(strString,"#","\#", 1, -1, 1)
        strString = replace(strString,"?","\?", 1, -1, 1)
        strString = replace(strString,"'","\'", 1, -1, 1)
        strString = replace(strString,"""","\""", 1, -1, 1)
        strString = replace(strString,"select","\select", 1, -1, 1)
        strString = replace(strString,"insert","\insert", 1, -1, 1)
        strString = replace(strString,"update","\update", 1, -1, 1)
        strString = replace(strString,"delete","\delete", 1, -1, 1)
        strString = replace(strString," or "," \or ", 1, -1, 1)
        strString = replace(strString," and "," \and ", 1, -1, 1)
        strString = replace(strString,"drop","\drop", 1, -1, 1)
        strString = replace(strString,"union","\union", 1, -1, 1)
        strString = replace(strString,"into","\into", 1, -1, 1)

        'Return cleaned value.
        SQLClean = Trim(strString)

    End If
End Function

i am not a MySQL guy, but i suggest using parametrized stored procedures serves as the best practice for preventing sql injection attack. — Zia, Sep 05 '12 at 06:12
@Zia could you explain a little more what you mean by parametrized stored procedures? Maybe provide a small example or something? Thanks!! — Frank G., Sep 05 '12 at 06:25
i cannot post a sample in comment, so i am posting a link http://support.microsoft.com/kb/164485 — Zia, Sep 05 '12 at 09:02

score 15 · Accepted Answer · edited May 23 '17 at 12:02

15

Please, DO NOT under any circumstances attempt to write your own SQL escaping code unless it is purely an academic exercise. You will get it wrong. If someone uses a SQL injection attack tool on your site you will suffer severe consequences. Businesses and careers have been destroyed by people taking a casual approach to this.

It took me all of three minutes to find an example on StackOverflow talking about Classic ASP and MySQL queries using parameters.

Please, please, please use the official facilities and do not roll your own.

edited May 23 '17 at 12:02

Community

1
1

answered Sep 05 '12 at 06:59

tadman

208,517
23
234
262

Well help me out here. What is it that an official facilities will do that I can't? And what kind of things do they do? And why can't I or anyone else write there own SQL escaping code specially if there person is a fairly good SQL/php, .net, asp or whatever coder? What's the difference between them vs some official facility? Thanks! – Frank G. Sep 05 '12 at 07:10
Personally I am writing my own and not worried about it because the site is IP specific so no one can get to it without an approved IP address. The SQL part is just an added security. And while doing it. It got me thinking and that's why I posted it just to see what people suggest or say! – Frank G. Sep 05 '12 at 07:12
1

Even if it's a web site for your grandmother you should be careful when writing SQL and do it properly. It's too easy to develop bad habits that will later come back to haunt you in a serious way. If you learn to use the proper tools, you'll be much better off. – tadman Sep 05 '12 at 14:09
1

I completely understand and that's why I'm asking here to learn and become a better programmer so I can hope to avoid those issues. – Frank G. Sep 05 '12 at 15:12
It's always good to understand what's going on internally, of course. Your solution was destined to end up somewhere like [The Daily WTF](http://thedailywtf.com/Comments/SQL-MUGging.aspx?pg=3) if it had been released into the wild. – tadman Sep 05 '12 at 15:35
1

@FrankG.: _not worried about it because the site is IP specific so no one can get to it without an approved IP address..._ -- Yeah, all an attacker has to do to bypass your homebrew technique is [spoof their IP address](https://en.wikipedia.org/wiki/IP_address_spoofing). Parent is dead on: Do. Not. Roll. Your. Own. – BryanH Nov 18 '12 at 04:26
Never roll your own unless you're an expert! – Nubcake Apr 04 '17 at 21:42

nageeb · Answer 2 · 2014-03-11T18:28:22.067

-1

Here's a good link to read up on for preventing SQL Injection Attacks in ASP Classic scripts.

It's also worth noting that you should always validate your variables, checking for proper values prior to dumping them in an SQL query. Checking for valid values is usually easier than checking for all the possible bad things that people can cram into the variables.

edited Mar 11 '14 at 18:28

answered Sep 05 '12 at 05:34

nageeb

2,002
1
13
25

Thank you for the info. But what if they try to run commands like delete, drop, union etc... will changing ' to '' (two single quotes) that stop it? – Frank G. Sep 05 '12 at 05:40
1

yes, because the commands will be located within the values of one of the submitted variables, and so the quotes of your script will mean they will be translated into literal values. – nageeb Sep 05 '12 at 05:42
So adding all the addition info I have in my function is a waste then? And there's nothing else I need to do to protect myself from it? – Frank G. Sep 05 '12 at 05:50
1

It's certainly not a waste, and I'm not going to tell you to remove it, since SQL injection attackers can be really clever... however, double-quoting quotes should be all you need. I also edited my answer to include a note on properly validating your variables (i.e checking for proper length, value types etc) which will help you far more than most replace() statements ever will. – nageeb Sep 05 '12 at 06:00
I do, prior to dumping to the db I check to insure there is data in them, what formats and so on. If needed I will also double check variables for data such as numbers and so on. However I also do it client side but sometimes client side isn't so reliable. Thank you very much. For now I'm going to keep the SQLClean like you said it can't hurt and any extra protection in today's world is a good think. Beside I have a function that cleans the data back on of the db to. Thank you again for your help!! – Frank G. Sep 05 '12 at 06:07
In MySQL that single replace definitely is not enough on its own. See the document linked from [the answer here](http://stackoverflow.com/questions/139199/can-i-protect-against-sql-injection-by-escaping-single-quote-and-surrounding-use/139810#139810) `the "\'" sequence will be translated to "\''". This will be understood by MySQL as: "\'" (escaped quote) followed by ' (unescaped quote).` – Martin Smith Sep 05 '12 at 06:42
@MartinSmith You are right. I will fix my answer to replace the single-quote to escaped single-quote. – nageeb Sep 05 '12 at 06:48

SQL Injection Is this Good

2 Answers2