Hello,
Can anyone suggest code to prevent a user from hacking a site thru the url query string?
As it is, I'm passing an integer as a variable thru the url to index.php. Based on the integer, index.php sets parameters, checks that files exist, then includes the appropriate files to display results to the user. Typical framework, just want to make sure I secure it from the beginning...
Thanks!
The best way to validate input is to make sure it's exactly what you're expecting. The more specific, the better. For example, if you're expecting a number, use is_numeric(). Better yet, if you're expecting an integer, do something like this:
if(!ereg("^[0-9]+$",$number)) exit("Invalid number!");And no matter what data type, if you're even remotely planning on using it in query - always, always, ALWAYS use mysql_real_escape_string. I can't stress that enough. Always. ALWAYS. Get my drift?
The reason for that is because of SQL injection, which can get to be a really ugly thing. Using mysql_real_escape_string is the best way to stop it dead in it's tracks.
Thanks again Trs2988, you seem to respond often to my dumbass questions...
Just a few dumbass questions about the syntax of your example, which works great:
So what does the positioning of ^ do for the statement, and what does the +$ do in this case?
Would it be a good idea to limit acceptable integer values, like check if it's between 1 and 12, etc?
And, how do people hack URL’s exactly?
Thanks!
Here are some answers.
1.
My syntax was "[0-9]+$" Any caret ^ that is NOT inside of brackets [] means the beginning. So, first I'm matching the beginning of the string. Then a number from 0-9, at least once (which is what the plus sign + means). And a dollar sign $ is the opposite of a caret , meaning the end. So: Beginning of string, a digit at least once, and then end of string. Any non-integer will return FALSE.
Their syntax was: "[0-9]" When inside of the brackets [], a caret ^ means "not". So literally, you're matching anything that is NOT 0-9. So, if you have a non-integer, this ereg will return TRUE.
It just depends on your preference. As far as I can tell, those two functions will do exactly the same thing (except, of course that one returns FALSE when something is wrong and the other returns TRUE).
2.
Usually, I don't limit the integer. The reason for this is that I like to let the database limit the integer. If I add a new row in the database (with a number of 13, for example), I don't want to have to change the code too. So what I do is after I'm sure I have an integer, I check and made sure it's a valid number in the database. Like this:
if(!ereg("^[0-9]+$",$id)) exit("Invalid ID!");
$result = mysql_query("select id from table_name where id='$id'");
if(!mysql_num_rows($result)) exit("That ID does not exist.");
That way, you're sure you have an integer, and a valid integer at that.
3.
One of the best places where I learned about SQL injection and stuff was hackthissite.org. Even though it doesn't really have plentiful reference material for how to do it, there are situations that they give you, and every one is breakable. So you really learn how to think like a hacker, and therefore how to protect yourself.
But what's the basic idea? Let me explain.
Let's say... you have a page that allows you to view/edit your contact data. This is common on lots of websites where you can have an account. So, that page would be something like editprofile.php?accountid=8, where 8 is the ID number for that account. And here's some sample code that's used to retrieve the contact data from the database.
if(empty($_GET['accountid'])) exit("You must have an account ID!");
$accountid = $_GET['accountid'];
$result = mysql_query("select firstname,lastname,address,phonenumber,email,etc1,etc2 from contact_info where accountid='$accountid'");
// retrieve and echo out account information, etc.
Seems nice and safe, right? The $accountid is in quotes, so nothing can happen to it... right?
What if some evil hacker goes to this address: editprofile.php?accountid=8' or accountid='6
Now, put it in context of the query:
select firstname,lastname,address,phonenumber,email,etc1,etc2 from contact_info where accountid='8' or accountid='6'
The owner of that website has a big problem. The evil hacker can now view and probably edit the private information for the person whose account number is 6. That's why it's critical to use mysql_real_escape_string, which would have escaped the single quotes that the evil hacker put into the address bar, and there wouldn't have been a problem.
Obviously, this is just a simple example. There are many other things that can damage your site with SQL injection, and a lot of people don't realize it.
You've got a big problem.
dumbass wrote:2. Would it be a good idea to limit acceptable integer values, like check if it's between 1 and 12, etc?
It depends on exactly what you are trying to do. If you're looking up a database, then maybe let the database handle it. If you plan on just using the integer as part of a filename, check that the file based on that value exists. If you're using it as a switch in a case statement, then the "default" can handle it.
On the other hand, if you're accepting an integer for processing later on by another system, and you know that the other system will only accept 1,2,3 then you should make sure that that's all you accept.
What's worth considering is what to do if these tests fail. If someone passes in a value that you don't like, I wouldn't give them an error response, I would substitute a value that I do like. So, if you have a script called showpage.php which accepts a variable called "page" which specifies what to display - maybe with values of ("home", "news", "faq"), and someone passes in a "page" value of "/etc/passwd", I wouldn't say "Error: Bad Page", I would just replace "/etc/passwd" with "home" and move on.
3. And, how do people hack URL’s exactly?
It can be as simple as noticing that when they login they go to "loggedin.php?uid=123" and they then request "loggedin.php?uid=246". If you're not doing basic context checking, this could allow them into someone else's account.
Or, they might spot a hidden field on a shopping cart form which says "<input type=hidden name=price value=99.99>" so they copy the form, change the value of this field to 9.99, and submit it.
These are basic examples, but I hope you get the idea. There are more complex examples, but the principle is always the same. And your defences should always be the same.
1. Don't design your system to require any more detail to be passed from the client than is absolutely necessary. Always use server side data where possible.
2. When you have to accept something from the client side, assume its malicious, test it rigorously, and react appropriately when it is a threat.
Better yet, if you're expecting an integer
... use [man]ctype_digit/man.
Would it be a good idea to limit acceptable integer values, like check if it's between 1 and 12, etc?
If the required range is known (and exists at all), then it does make sense to include a range check in the input validation.
You might want to take a look at what the PHP Manual has to say concerning security[/man].
Wow, thanks for the responses folks! Great info! I learned a great deal from this post alone!
One other question, tho maybe I should start a new thread for this, but...
As I mentioned above, I'm using an integer to process user actions thru the URL, I want to use that integer to dynamically create file names to load, but for reasons of ease of managing these include files, I don't want to name all my .inc files after integers, I'd rather name them descriptively, like "who.inc", "contact.inc", etc.
So I was thinking of storing the descriptive part of the file name, like "who", "contact", in an array, and then using the URL integer to index the array to grab the file name. Is this sound?
By doing this I figure that I'd only have to call the require() function once for the content part of the page, as opposed to using require() many times in a switch. I've heard that using a bunch of includes in a switch can be bad for server performance, but I've also read somewhere that dynamically assembling file names can be dangerous.
Any thoughts? Thanks!
I've seen this suggestion several times now and have a question, so does no one really use addslashes() anymore? Is using mysql_real_escape_string() the preferred method of preventing sql injection attacks?
The problem with dynamicly generated filenames is this. What people should avoid doing is having a URL like somephpfile.php?include=someincludefile.php. And then in their script, they require($_GET['include']). As you can probably see, that could easily be abused to read in almost any file on the computer, and it's a huge security problem. That's a simple example; there are other, more subtle examples.
But if you have an array(1=>'thisfile.php', 2=>'thatfile.php', 3=>'etcetc') and then just take the integer by URL, you shouldn't have a problem. Just make sure your $_GET value is an integer and that it's in the array (array_key_exists()). I don't really see a security threat in that at all.
hadoob024 wrote:I've seen this suggestion several times now and have a question, so does no one really use addslashes() anymore? Is using mysql_real_escape_string() the preferred method of preventing sql injection attacks?
I use the two interchangeably. I believe they're both equally safe.
