function to provide some extra security in php with query_string

Question

some years ago I started using the following code including in the top of my pages. I read that was good and used it. But I was wondering, is it helpful?

$page = "index.php";
$cracktrack = $_SERVER['QUERY_STRING'];
$wormprotector = array('chr(', 'chr=', 'chr%20', '%20chr', 'wget%20', '%20wget', 'wget(',
 'cmd=', '%20cmd', 'cmd%20', 'rush=', '%20rush', 'rush%20',
 'union%20', '%20union', 'union(', 'union=', 'echr(', '%20echr', 'echr%20', 'echr=',
 'esystem(', 'esystem%20', 'cp%20', '%20cp', 'cp(', 'mdir%20', '%20mdir', 'mdir(',
 'mcd%20', 'mrd%20', 'rm%20', '%20mcd', '%20mrd', '%20rm',
 'mcd(', 'mrd(', 'rm(', 'mcd=', 'mrd=', 'mv%20', 'rmdir%20', 'mv(', 'rmdir(',
 'chmod(', 'chmod%20', '%20chmod', 'chmod(', 'chmod=', 'chown%20', 'chgrp%20', 'chown(', 'chgrp(',
 'locate%20', 'grep%20', 'locate(', 'grep(', 'diff%20', 'kill%20', 'kill(', 'killall',
 'passwd%20', '%20passwd', 'passwd(', 'telnet%20', 'vi(', 'vi%20',
 'insert%20into', 'select%20', 'nigga(', '%20nigga', 'nigga%20', 'fopen', 'fwrite', '%20like', 'like%20',
 '$_request', '$_get', '$request', '$get', '.system', 'HTTP_PHP', '&aim', '%20getenv', 'getenv%20',
 'new_password', '&icq','/etc/password','/etc/shadow', '/etc/groups', '/etc/gshadow',
 'HTTP_USER_AGENT', 'HTTP_HOST', '/bin/ps', 'wget%20', 'unamex20-a', '/usr/bin/id',
 '/bin/echo', '/bin/kill', '/bin/', '/chgrp', '/chown', '/usr/bin', 'g++', 'bin/python',
 'bin/tclsh', 'bin/nasm', 'perl%20', 'traceroute%20', 'ping%20', '.pl', '/usr/X11R6/bin/xterm', 'lsof%20',
 '/bin/mail', '.conf', 'motd%20', 'HTTP/1.', '.inc.php', 'config.php', 'cgi-', '.eml',
 'file://', 'window.open', '<SCRIPT>', 'javascript://','img src', 'img%20src','.jsp','ftp.exe',
 'xp_enumdsn', 'xp_availablemedia', 'xp_filelist', 'xp_cmdshell', 'nc.exe', '.htpasswd',
 'servlet', '/etc/passwd', 'wwwacl', '~root', '~ftp', '.js', '.jsp', 'admin_', '.history',
 'bash_history', '.bash_history', '~nobody', 'server-info', 'server-status', 'reboot%20', 'halt%20',
 'powerdown%20', '/home/ftp', '/home/www', 'secure_site, ok', 'chunked', 'org.apache', '/servlet/con',
 '<script', '/robot.txt' ,'/perl' ,'mod_gzip_status', 'db_mysql.inc', '.inc', 'select%20from',
 'select from', 'drop%20', '.system', 'getenv', 'http_', '_php', 'php_', 'phpinfo()', '<?php', '?>', 'sql=');
$checkworm = str_replace($wormprotector, '*', $cracktrack);
if ($cracktrack != $checkworm){
 $cremotead = $_SERVER['REMOTE_ADDR'];
 $cuseragent = $_SERVER['HTTP_USER_AGENT'];
 header("location:$page");
 die();
}

Answer 1

If you sanitize all user input properly, there's absolutely no need to use a script like this.

Besides that, it's also case sensitive (str_replace vs str_ireplace) which means that I can easily bypass it by making use of a mix of uppercase and lowercase letters. It also only checks the query string, useless against POST requests.

Answer 2

Although it's great that you're concerned about security, and you're following the principle of treating all input with suspicion, I don't think that list is terribly useful.

It's a rather arbitrary selection of potentially unwanted strings/commands/tags/folder names and other things. It's likely to get out of date over time, and probably is already. Having a generic list like this is never going to catch everything, and may also lend a false sense of security that your application is secure when really it's not.

As another answer has already mentioned, you want to be checking each input you get from your application (whether via query string variables, POST variables or wherever) and validating that it meets your expectations (e.g. if you're expecting a numeric value, is the value passed in numeric?).

Then if you plan to redisplay or re-use that data, you might want to sanitise if further, and strip out things that might potentially be dangerous in the context where it will be used. For example, you might strip out "script" tags if you're going to display the data on a web page.

Answer 3

In general, I personally wouldn't use this strategy. I'd rather sanitize each and every input. If a user passes .bash_history in the URL I don't care because it's never going to do anything in my script.

I could maybe see something like this being useful if you had some third-party low reliability script that was available for anyone to hit. Even in that scenario though it seems like a semi-reliable band-aid at best.

For applications you write however, this should hopefully be unnecessary.