Extra secure way to protect your pages using strpos?

[TimTimTimma]

I was in search of a method to search the browser parameters for all of the program execution functions. I did alittle toying around and I came up with this..It works pretty good, but I am unsure of how exactly one does sql injections, and other things like that through the browser parameters..So I figured I would ask some of you here:

Can this script be trusted to protect me against a more broad range of hacking attempts then without it. Not neccesarily all, or even ALOT, but am I at a higher risk without it than I am with it? Is it worth adding it to my CMS backend code?

<?php
//********************************************************************************************
//
// Name: Suspect Filter
//
// Description: Searchs browser parameters for preset arrays containing all of php's program 
// execution functions, aswell as the mysql_query function. If finds any of these functions it 
// kills the script  immediately. It also searches for mysql commands,  on the chance it finds 
// one, it will log the find, and on the third find it kills the script.
//
// IMPORTANT NOTE: You can insert the name of mysql tables you wish to protect into the 
// $illegal_uri['keywords']
//
// Version: 2.1.0
// Scripted by: Timothy Hensley (TimTimTimma)
//
//*********************************************************************************************

$log_location = dirname(__FILE__)."/log.txt";
global $log_location;
$illegal_functions = array("truncate", "alter", "mysql_query(", "base64_encode(", "base64_decode(", "escapeshellarg(", "exec(", "passthru(", "proc_closes(", "proc_get_status(", "proc_nice", "proc_open(", "proc_terminate(", "shell_exec(", "system(");
$illegal_keywords = array("set", "drop", "where", "insert", "select", "table1", "table2", "table3");

$total_illegal_functions = count($illegal_functions);
$total_illegal_keywords = count($illegal_keywords);

$filter = array("$", "*", ";");

$browser_uri = str_replace($filter, "x", addslashes(strtolower($_SERVER['REQUEST_URI'])));

function log_suspect($suspect, $type = ""){
	if(@file_exists($GLOBALS['log_location']) !== TRUE){
		die("Either you gave the wrong location of the log file, or it does not exist.");		
	}	
	$hand = @fopen($GLOBALS['log_location'], "a");	
	if(isset($type)){
		$suspected = "Suspicious Function: ".$suspect;
	}else{
		$suspected = "Suspicious Keyword: ".$suspect;
	}
	$log_content = 
	"
	Date: ".date("F j, Y, g:i:s a")."
	IP: ".$_SERVER['REMOTE_ADDR']."
	".$suspected."		
	Action: http://".$_SERVER['HTTP_HOST'].$_SERVER['REQUEST_URI']."
	";

if(!@fwrite($hand, $log_content)){
	die("There appears to be a file permission error. You must chmod the log file to 0602");
}	
}

for($i = 0; $i <= $total_illegal_functions; $i++){
	if(strpos($browser_uri, $illegal_functions[$i]) !== FALSE){
		log_suspect($illegal_functions[$i], TRUE);
		$suspects = 3;
	}
}

for($i = 0; $i <= $total_illegal_keywords; $i++){
	if(strpos($browser_uri, $illegal_keywords[$i]) !== FALSE){
		log_suspect($illegal_keywords[$i]);
		$suspects++;	
	}
}

if($suspects >= 3){
	die("HACKING ATTEMPT!");
}
?>

[mrhappiness]

TimTimTimma wrote:

Can this script be trusted to protect me against a more broad range of hacking attempts then without it[/code]

what attempts do you think this code can protect you from?

[TimTimTimma]

I am unsure, I was under the impression that functions and sql queries could somehow be injected into the page through the browser. Was I led on wrong? Is that code worthless?

[mrhappiness]

You made up your mind and so it was not worthless 🙂

you can perhaps become a victim of sql injections and other baddies but only if you don't consider a few simple things

1. Never trust incoming data

2. Don't use register_globals

3. Initialise your vars

4. if you expect an integer, make it an integer

   $_GET['id'] = (int)$_GET['id'];[*]for working with strings inside mysql-queries: [man]mysql_real_escape_string[/man] (if [man]get_magic_quotes_gpc[/man] returns false)[/list]

[TimTimTimma]

if I am expecting an integer I always check to make sure it has a numeric value with is_number() :-D

[mrhappiness]

17.54 is a number as well 😉

[laserlight]

but I am unsure of how exactly one does sql injections, and other things like that through the browser parameters

Basically, the idea is to break out of user input and begin one's own SQL statement. This usually involves closing the current string with one's own single quote, and commenting out the rest of your SQL statement with --.

Can this script be trusted to protect me against a more broad range of hacking attempts then without it.

What you're doing is trying to find potentially dangerous keywords, and then declaring a 'hacking attempt' is any are found. This is an "allow all, deny some" method - it may work, but if you dont deny enough, you lose.
It is better to use a "deny all, allow some" method. In this case, it consists of ensuring that special characters are escaped properly, especially single quotes. To avoid SQL injection, ensure that single quotes in incoming data are escaped. This means doubling them, or in the case of MySQL, prefixing them with a backslash.

Not neccesarily all, or even ALOT, but am I at a higher risk without it than I am with it?

What happens if you match valid data? Maybe some where along the line a user wants to search for info on a certain operating system, e.g. "system v"? Or perhaps they are looking for the "executives" of a certain company?

if I am expecting an integer I always check to make sure it has a numeric value with is_number() :-D

If you want to check if an incoming variable is an integer, use [man]ctype_digit/man.

[Shrike]

As laserlight pointed out, you missed these for a kickoff

truncate
alter
drop

Also users cannot execute PHP functions just by including them in the query string (the whole name including parentheses will be treated as a string). You'd have to do something suicidal like:

$func = $_GET['func'];
$func();

With regard to database integrity it would be better to restrict the priviledges of the user which PHP logs into the database as (say just allow INSERT UPDATE and DELETE - or the absolute minimum necessary).

As an aside write any system level errors to a log file and not to screen. Calling some innocent user a hacker because your code had a bug will not endear them to your website 🙂

[mrhappiness]

i don't see ne need for checking for drop, alter, delete, ...

why?

$sql = "SELECT user_id
FROM usertable
WHERE pass = MD5('".$_POST['pass']."') AND nick = '".$_POST['nick']."'";

will always be a SELECT which can not modify neither the database nor the tables nor the tables' content

if perhaps might become

SELECT user_id
FROM usertable
WHERE pass = MD5('something') AND nick = '1' OR '1' = '1'

which would allow logging in using wrong credentials but with the use uf mysql_real_escape_string it will be

SELECT user_id
FROM usertable
WHERE pass = MD5('something') AND nick = '1\' OR \'1\' = \'1'

which won't do any harm at all

so what do i miss?

@
you really have a hoster allowing you to create mutiple mysql users and granting different permissions to them?

[Shrike]

At the most severe SQL injection attacks will end the current SQL statement and start a new one. I don't know the particulars but something along the lines of

// URL format: yourscript.php?string=somestring;truncate users
$qry = "UPDATE users SET column = $_GET['string']";

The examples I've found are for SQL Server, although I think recent MySQL versions can execute multiple commands in one statement - could be wrong here

Any reasonable host should allow you that sort of capability. For the sort of money my company charges we damn well should allow clients to do whatever the hell the like with their database 😉

[mrhappiness]

Shrike wrote:

could be wrong here

you are 🙂
[man]mysql_query[/man] executes only one single statement and if php adds support to pass multiple statements to mysql_query separated by ; i'm quite sure they will extend mysql-real_escape_string to mask the ; as well...

Any reasonable host should allow you that sort of capability. For the sort of money my company charges we damn well should allow clients to do whatever the hell the like with their database 😉

anyway, if you have to do an UPDATE you need a mysql user who is able to do UPDATEs and so i can do mysql injection executing an UPDATE as well (if mysql_real_escape_string wouldn't exist)

[Shrike]

But not everyone uses MySQL with PHP. Most, but not all 😉

[Weedpacket]

For example, MS SQL Server, as Shrike already noted; I've seen SQL insertion attacks there that allow execution of arbitary code on the attacked system. Joy! The things you can get up to with a judiciously-placed '.

[TimTimTimma]

mrhappiness wrote:

i don't see ne need for checking for drop, alter, delete, ...

why?

$sql = "SELECT user_id
FROM usertable
WHERE pass = MD5('".$_POST['pass']."') AND nick = '".$_POST['nick']."'";

will always be a SELECT which can not modify neither the database nor the tables nor the tables' content

if perhaps might become

SELECT user_id
FROM usertable
WHERE pass = MD5('something') AND nick = '1' OR '1' = '1'

which would allow logging in using wrong credentials but with the use uf mysql_real_escape_string it will be

SELECT user_id
FROM usertable
WHERE pass = MD5('something') AND nick = '1\' OR \'1\' = \'1'

which won't do any harm at all

so what do i miss?

@
you really have a hoster allowing you to create mutiple mysql users and granting different permissions to them?

I do aswell, I have a reseller account with mediacatch.com

===============new version of script====================

<?php

//********************************************************************************************
//
// Name: Suspect Filter
//
// Description: Searchs browser parameters for preset arrays containing all of php's program 
// execution functions, aswell as the mysql_query function. If finds any of these functions it 
// kills the script  immediately. It also searches for mysql commands,  on the chance it finds 
// one, it will log the find, and on the third find it kills the script.
//
// IMPORTANT NOTE: You can insert the name of mysql tables you wish to protect into the 
// $illegal_uri['keywords']
//
// Version: 2.1.0
// Scripted by: Timothy Hensley (TimTimTimma)
//
//*********************************************************************************************

$log_location = dirname(__FILE__)."/log.txt";
global $log_location;
$illegal_functions = array("truncate", "alter", "mysql_query(", "base64_encode(", "base64_decode(", "escapeshellarg(", "exec(", "passthru(", "proc_closes(", "proc_get_status(", "proc_nice", "proc_open(", "proc_terminate(", "shell_exec(", "system(");
$illegal_keywords = array("set", "drop", "where", "insert", "select", "table1", "table2", "table3");

$total_illegal_functions = count($illegal_functions);
$total_illegal_keywords = count($illegal_keywords);

$filter = array("$", "*", ";");

$browser_uri = str_replace($filter, "x", addslashes(strtolower($_SERVER['REQUEST_URI'])));

function log_suspect($suspect, $type = ""){
	if(@file_exists($GLOBALS['log_location']) !== TRUE){
		die("Either you gave the wrong location of the log file, or it does not exist.");		
	}	
	$hand = @fopen($GLOBALS['log_location'], "a");	
	if(isset($type)){
		$suspected = "Suspicious Function: ".$suspect;
	}else{
		$suspected = "Suspicious Keyword: ".$suspect;
	}
	$log_content = 
	"
	Date: ".date("F j, Y, g:i:s a")."
	IP: ".$_SERVER['REMOTE_ADDR']."
	".$suspected."		
	Action: http://".$_SERVER['HTTP_HOST'].$_SERVER['REQUEST_URI']."
	";

if(!@fwrite($hand, $log_content)){
	die("There appears to be a file permission error. You must chmod the log file to 0602");
}	
}

for($i = 0; $i <= $total_illegal_functions; $i++){
	if(strpos($browser_uri, $illegal_functions[$i]) !== FALSE){
		log_suspect($illegal_functions[$i], TRUE);
		$suspects = 3;
	}
}

for($i = 0; $i <= $total_illegal_keywords; $i++){
	if(strpos($browser_uri, $illegal_keywords[$i]) !== FALSE){
		log_suspect($illegal_keywords[$i]);
		$suspects++;	
	}
}

if($suspects >= 3){
	die("HACKING ATTEMPT!");
}

?>


You made it to the page, everyting seems AOK!
<br /><br />
<?
echo $browser_uri ." - ". $suspects;
?>

A close look at the log file revealed alittle bit of info about the attempts

Date: July 19, 2005, 4:23:01 pm
IP: 24.166.3.xxx
Suspicious Function: where		
Action: phpublisher.net/test/test_security.php?find=delete%20from%20table1%20where%20this%20=%20that;

Date: July 19, 2005, 4:23:01 pm
IP: 24.166.3.xxx
Suspicious Function: table1		
Action: phpublisher.net/test/test_security.php?find=delete%20from%20table1%20where%20this%20=%20that;

Date: July 19, 2005, 4:24:43 pm
IP: 24.166.3.xxx
Suspicious Function: where		
Action: phpublisher.net/test/test_security.php?find=select%20*%20from%20table1%20where%20this%20=%20%27that%27

Date: July 19, 2005, 4:24:43 pm
IP: 24.166.3.xxx
Suspicious Function: select		
Action: phpublisher.net/test/test_security.php?find=select%20*%20from%20table1%20where%20this%20=%20%27that%27

Date: July 19, 2005, 4:24:43 pm
IP: 24.166.3.xxx
Suspicious Function: table1		
Action: phpublisher.net/test/test_security.php?find=select%20*%20from%20table1%20where%20this%20=%20%27that%27	

Date: July 19, 2005, 4:26:00 pm
IP: 24.166.3.xxx
Suspicious Function: exec(		
Action: phpublisher.net/test/test_security.php?find=exec(

I think this is MUCH better, another great thing about this, is that you can actually protect your tables with this method too, you can list specific tables you would like to protect. i think this is pretty sharp, anymore opinions?

[bubblenut]

TimTimTimma wrote:

I think this is MUCH better, another great thing about this, is that you can actually protect your tables with this method too, you can list specific tables you would like to protect. i think this is pretty sharp, anymore opinions?

I'm afraid I would have to disagree that this is good method. It fails to address the fact that the underlying code may be inherently insecure and just protects it with an extra layer. Any erroneous methods used in the underlying code which would make this layer necesarry are still there and still hold the potential to cause problems should a malicious user find another way of getting their data into the system. IMHO SQL Injection attacks should be addressed at the point where SQL is executed or in a layer imediately wrapping that point so that some context may be imposed.

[planetsim]

Is it just me or is it that trying to predict every possible way a person can break into a site stupid? More to the point, dont predict what someone may do. You should only allow certain data into the database or where ever. E.g. if you want only numeric data you can be cheap and use a cast of type (int), this doesnt actually validate rather if there are non-numeric characters they are thrown away and any numeric characters are saved and used when required. (you may wish to use [man]is_numeric[/man] instead)

From there we can also create our own function which deals with strings. Now strings can contain Drop, Select, Update, Truncate believe it or not I havent tested your function or really gone over it so posting of data may not be affected. Valid Strings can be anything really

<"?][()+=`~!@#$%^&*

And more are all valid however backticks and single/double quotations arent going to be valid (double quotes should actually be alright but it will depend on how you use it its best to be safe) So as they are still valid characters we dont want to remove them but add something extra to them such as an

\

you can use [man]addslashes/man of course if [man]magic_quotes_gpc[/man] are on we dont need to but I like to check strip all of them and start at a clean state (better for portablility). Of course you can use [man]mysql_real_escape_string[/man] (as [man]mysql_escape_string[/man] is deprecated), instead of using addslashes either way they will work in similar ways. So far we only dealt with integers and strings most will fall under the same category but there will be some you may want to go that extra mile more to validate against which will probably more or less require you to know what format they are in.

Now although I havent used your code for myself Im going to assume your site uses it. I did my own little test and reaction time compared to the rest of the site was noticablly slower.

Just my $0.02 for now

[Shrike]

Bubblenut hit the nail on the head. Deal with data itself, not try to understand what it is trying to do. The following is borrowed from php.net (I think)

	/**
	 * Prepare data for the database
	 * @param string $input
	 * @return string
	 */
	function prep( $input )
	{
		// Revert characters back to their original form
		$input = html_entity_decode( $input );
		if(get_magic_quotes_gpc() )
		{
			$input = stripslashes( $input );
		}
		// If not numeric escape string for MySQL
		if(!is_numeric( $input ))
		{
			$input = mysql_real_escape_string($input);
		}
		return $input;
	}
//e.g.
$field = $sql->prep( $_POST['field'] );

I call the above function for each and every bit of untrusted piece of data which is going to be inserted into my database. Since this will allow HTML to be written to the database you should use html_entities() before displaying to prevent session hijacking etc.

Further to what planetsim is saying, if you are expecting an integer value for a particular field, ensure that the database column type is an INT of the correct length.

[TimTimTimma]

Shrike wrote:

Bubblenut hit the nail on the head. Deal with data itself, not try to understand what it is trying to do. The following is borrowed from php.net (I think)

	/**
	 * Prepare data for the database
	 * @param string $input
	 * @return string
	 */
	function prep( $input )
	{
		// Revert characters back to their original form
		$input = html_entity_decode( $input );
		if(get_magic_quotes_gpc() )
		{
			$input = stripslashes( $input );
		}
		// If not numeric escape string for MySQL
		if(!is_numeric( $input ))
		{
			$input = mysql_real_escape_string($input);
		}
		return $input;
	}
//e.g.
$field = $sql->prep( $_POST['field'] );

I call the above function for each and every bit of untrusted piece of data which is going to be inserted into my database. Since this will allow HTML to be written to the database you should use html_entities() before displaying to prevent session hijacking etc.

Further to what planetsim is saying, if you are expecting an integer value for a particular field, ensure that the database column type is an INT of the correct length.

Then, wouldn't this be a good way to validate EVERY $GET/$POST var?

function prep($input)
{
	$input = html_entity_decode($input); 
	if(get_magic_quotes_gpc()){ 
		$input = stripslashes($input); 
	}
	if (is_numeric($input)) {
       $input = mysql_real_escape_string($input);
   	}
	return $input; 
} 

if(count($_POST) > 0){
	foreach ($_POST as $var => $value) {	
		$_POST[$var] = prep($_POST[$var]);

}
}

Assuming only post data is used to insert information into the mysql database..

[Weedpacket]

if (is_numeric($input)) {
       $input = mysql_real_escape_string($input);
       } 

If $input is numeric, there won't be any characters that need escaping....

[Shrike]

You've missed the ! when copying and pasting 🙂