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)
//
//*********************************************************************************************
Can this script be trusted to protect me against a more broad range of hacking attempts then without it[/PHP]what attempts do you think this code can protect you from?
TimTimTimma
07-19-2005, 04:52 AM
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
07-19-2005, 05:01 AM
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
Never trust incoming data Don't use register_globals Initialise your vars if you expect an integer, make it an integer[php]$_GET['id'] = (int)$_GET['id']; for working with strings inside mysql-queries: mysql_real_escape_string (if get_magic_quotes_gpc returns false)
TimTimTimma
07-19-2005, 05:07 AM
if I am expecting an integer I always check to make sure it has a numeric value with is_number() :-D
mrhappiness
07-19-2005, 05:09 AM
17.54 is a number as well ;)
laserlight
07-19-2005, 05:36 AM
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 ctype_digit().
Shrike
07-19-2005, 05:55 AM
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
07-19-2005, 06:14 AM
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 beSELECT 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?
@Shrike
you really have a hoster allowing you to create mutiple mysql users and granting different permissions to them?
Shrike
07-19-2005, 06:28 AM
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
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
07-19-2005, 06:40 AM
could be wrong hereyou are :)
mysql_query 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
07-19-2005, 06:43 AM
But not everyone uses MySQL with PHP. Most, but not all ;)
Weedpacket
07-19-2005, 07:32 AM
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
07-19-2005, 02:25 PM
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 beSELECT 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?
@Shrike
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)
//
//*********************************************************************************************
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
07-20-2005, 04:23 AM
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
07-20-2005, 06:42 AM
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 is_numeric 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 addslashes() of course if magic_quotes_gpc 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 mysql_real_escape_string (as mysql_escape_string 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
07-20-2005, 06:50 AM
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
07-20-2005, 08:10 PM
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?
Assuming only post data is used to insert information into the mysql database..
Weedpacket
07-21-2005, 02:58 AM
if (is_numeric($input)) {
$input = mysql_real_escape_string($input);
}
If $input is numeric, there won't be any characters that need escaping....
Shrike
07-21-2005, 05:11 AM
You've missed the ! when copying and pasting :)
laserlight
07-21-2005, 07:41 AM
although I think recent MySQL versions can execute multiple commands in one statement
I dont think it has to do with MySQL itself (i.e. you can always commit multiple SQL statements), but with the interface that PHP provides.
e.g. it is true that mysql_query() only executes one SQL statement - but mysqli_multi_query() allows for multiple SQL statements.
cyberlew15
08-17-2005, 04:33 PM
because i am externally hosted do you think they mighty of stopped these kindof attacks at a application level I never knew this could be done
PHP Builder
Copyright WebMediaBrands Inc. All Rights Reserved.