Security holes.

[Sagefire135]

I am pretty close to brand new when it comes to coding, lucky for me I can also learn basics pretty fast though. So I have created a basic site (hosted locally) that has a registration, login/logout, pm system, forum, chat, and some admin controls. I cant find any functionality problems but it woudl never survive the internet because its got to be ripe with security holes (SQL Injections and such).

So I was wondering, is there a list someplace on here that sorta summarizes security problems and how to fix them? If not, what are some typical places that someone might find a hole and how would you fix them?

-Sage

[karnedge]

The first place to look would be anywhere the user interacts with the scripts itself. Usually this applies to database queries based on what the user inputs. As good practice, always use mysql_real_escape_string() on those variables just in case.

http://en.wikipedia.org/wiki/SQL_injection <-- Wikipedia's examples are fairly easy to understand.

Other inputs may include textareas where people would like to input HTML tags (especially javascript) or even PHP.

More so, I would be looking at URL verification with actual links AND images. XSS (Cross-Site Scripting) is the easiest with things like Avatars and such.

http://elegantcode.com/2009/05/28/cross-site-scripting-xss/ <-- They have fairly descent blog about prevention methods and ways to detect XSS scripts if already implanted. There are also a variety of other links related to the subject.

Hope that helps to start you off at least.

[Chris]

- SQL injection, always pass strings through mysql_real_escape_string()
- SQL injection, make sure all numbers are numbers; (int) conversion or '$var'.
- check for negative values in forms

The 3 above are enough (except for user provided content since this is a bit more tricky). For user generated content just strip all html tags and do not allow images, you can enalble them later once you learn how these works.

[yuppio]

For user provided content use htmlspecialchars() function and for SQL things mentioned above.

[Darklandz]

Here's a little function i use ....

Code: [Select]

<?php

function secureInput($string) {
 
$string = strip_tags($string);
$string = htmlspecialchars($string);
$string = trim($string);
$string = stripslashes($string);
$string = mysql_real_escape_string($string);

return $string;
}
?>


[Nox]

I think this one is useful only in specific situations, what if you want user to insert tags or be able to have spaces in the beginning of the input ... and stripslashes is useful imho only when there are magic quotes active

There's a very good article, unfortunately only in czech, but here's a Google's translation (not perfect, but the important parts are in php so it shouldn't be a problem):
http://translate.google.com/translate?u=http%3A%2F%2Fphpfashion.com%2Fescapovani-definitivni-prirucka&ie=UTF8&sl=cs&tl=en

Edit 1:
There is a large number of different threaths, search for:
SQL/PHP/Javascript injection
XSS
Clickjacking
CSRF
Session fixation
Session poisoning
(I won't remember all of them now, will modify eventually)

Well - it imho wouldn't be a bad thing to make a topic with list of all security issues known to us. Doesn't have to contain solutions,
just for everyone to know what terms to search

Edit 2:
Just discovered this: http://en.wikipedia.org/wiki/Category:Web_security_exploits
There are some more

[Crazy-T]

Here's a little function i use ....

Code: [Select]

<?php

function secureInput($string) {
 
$string = strip_tags($string);
$string = htmlspecialchars($string);
$string = trim($string);
$string = stripslashes($string);
$string = mysql_real_escape_string($string);

return $string;
}
?>


Isn't it slower doing it like that?.. why not just
return (strip_tags(htmlspecialchars(trim(stripslashes(mysql_real_escape_string($string))))));
Yeah some people might say it might look harder to read, but to me it doesn't, just looks the same.

- SQL injection, always pass strings through mysql_real_escape_string()
- SQL injection, make sure all numbers are numbers; (int) conversion or '$var'.
- check for negative values in forms

The 3 above are enough (except for user provided content since this is a bit more tricky). For user generated content just strip all html tags and do not allow images, you can enalble them later once you learn how these works.

SQL injection, make sure all numbers are numbers; (int) conversion or '$var'.

$_POST['number'] = (int) $_POST['var'];
I could add a - in there it will show that so you could do $_POST['number'] = abs((int) $_POST['var']); would work.?

[JGadrow]

I have a hard time understanding why so much effort is taken to 'clean' data for security measures. Instead, why not 'validate' the data and return an error if it is invalid?

For instance, instead of forcing data to be an unsigned integer, why don't you validate that it's an unsigned integer?

Code: (php) [Select]

if (!is_numeric ($someVar) || intval ($someVar) < 0)
{
    throw new ErrorException ('Why do I see this message? Should I be logging your activity to report as a hacking incident?', 0, E_ERROR, __FILE__, __LINE__);
}

[codestryke]

You hit the nail on the head JGadrow!

For years I've been using a function I call isnumber($number, $zerovalid = false) which simply returns true or false. I would say 98% of the time I need to check for a valid whole number that corresponds to a key value in the database. This is why I have the  $zerovalid defaulted to false because the database never starts with a key value of zero.

If it fails this check then we throw the user 'Invalid data selected'. To me this is just a cleaner way to process the data both for the user and for the next person that has to look at my code.

[shoespeak]

I have a hard time understanding why so much effort is taken to 'clean' data for security measures. Instead, why not 'validate' the data and return an error if it is invalid?

For instance, instead of forcing data to be an unsigned integer, why don't you validate that it's an unsigned integer?

Code: (php) [Select]

if (!is_numeric ($someVar) || intval ($someVar) < 0)
{
    throw new ErrorException ('Why do I see this message? Should I be logging your activity to report as a hacking incident?', 0, E_ERROR, __FILE__, __LINE__);
}

Great tip, I have used this is_numeric method for a while and it has worked great! As stryke mentioned the majority of the database queries seem to be checking numbers anyway.

However, I think it is important to still clean data, right when it comes from the user. Sometimes you may not know if the data you end up with is supposed to be a number or alphanumeric. Sometimes you want it to be both, a number followed by some text, etc. IMO the quicker you clean the data the better...less likely you will forget what is cleaned and what isn't (it is so annoying to discover those \\\' type bugs!)

[JGadrow]

However, I think it is important to still clean data, right when it comes from the user.

If you look closely, I stated that I'm not sure why people 'clean' data as a security measure. Sure, you validate the data to be sure it's free of corruption. For instance, if you're not able to be in control of your environment (due to workplace politics or whatever) and someone has magic_quotes turned on, then it's a good case for removing whatever has been added as an escaping mechanism.

However, there are other cases for this in a shared-code environment. Has code been run that has already used database functions to escape data that, for some reason (let's just say this for the sake of argument, it's not a realistic occurrence but similar situations can happen) you have a legitimate reason to alter the query written by another function. You will want to remove any escaping that has been performed on the data.

[Scion]

I think the underlying principle is that you can never be too paranoid.

Validate data that your going to use...... if your expecting the user to enter a date within the next two weeks then you should be checking server side that the data supplied represents a date within the next two weeks....

Clean data that your going to present back to the users.....character descriptions, avatar images, usernames etc...

Track/Log as much as you can

basically assume that your users are out to cheat where ever possible....

[Crazy-T]

I think the underlying principle is that you can never be too paranoid.

Validate data that your going to use...... if your expecting the user to enter a date within the next two weeks then you should be checking server side that the data supplied represents a date within the next two weeks....

Clean data that your going to present back to the users.....character descriptions, avatar images, usernames etc...

Track/Log as much as you can

basically assume that your users are out to cheat where ever possible....

avatar images

Couldnt you use this?

$allowed = array('gif', 'png', 'jpg', 'jpeg');
$check_image = strtolower(substr($variable_post_name, -3));
if(!in_array($check_image, $allowed)) {
  echo 'You cannot use this picture, invalid format. You can only use, gif, png, jpg, jpeg.<br />';
  exit;
}

[Helderic]

How do you guys remember to implement all these checks, etc? 
I always struggle to remember these. 

[JGadrow]

How do you guys remember to implement all these checks, etc?

Practice, practice, practice. And, honestly, there's STILL times that I forget even though I'm super-paranoid! lol

[karnedge]

@Helderic
One way is to use a database class that you use throughout your game that automatically cleans input when used.

[codestryke]

avatar images

Couldnt you use this?

$allowed = array('gif', 'png', 'jpg', 'jpeg');
$check_image = strtolower(substr($variable_post_name, -3));
if(!in_array($check_image, $allowed)) {
  echo 'You cannot use this picture, invalid format. You can only use, gif, png, jpg, jpeg.<br />';
  exit;
}
[/quote]

Absolutely NOT! With the above code I could create an EXE, rename the file extension and upload it to your server and it would successfully pass your check. Remember the mantra, never trust anything from the browser, not even a file name. To make sure you are getting an image use getimagesize. getimagesize returns an array of information, including what type of image it is.

[Harkins]

Or, worse, upload a js file. IE will execute Javascript from files included as the target of an image tag.

[JGadrow]

Not to mention there's a slight flaw in the logic of that statement even if it was a good idea. You're checking the last 3 characters of the file extension, yet you have a 4 character extension listed as 'allowed.' It would never match.

Here's a great read about upload security for those who are interested. It's quite involved.

[codestryke]

Or, worse, upload a js file. IE will execute Javascript from files included as the target of an image tag.

phpBB learned that exploit the hard way LOL

[Crazy-T]

Not to mention there's a slight flaw in the logic of that statement even if it was a good idea. You're checking the last 3 characters of the file extension, yet you have a 4 character extension listed as 'allowed.' It would never match.

Here's a great read about upload security for those who are interested. It's quite involved.

But who said i was on about uploading? .. I dont get them to upload it to my server they use other sites like imageshark/tinypic ect

[Nox]

But why not do it properly anyway?

[Crazy-T]

But why not do it properly anyway?

To true

[Scion]

But who said i was on about uploading? .. I dont get them to upload it to my server they use other sites like imageshark/tinypic ect

ahh but then there is a whole lot of other problems with that approach as well....can you guarantee that the remote server is allways going to serve you an image (its not enough to just test it once) and you have to worry about XSS attacks..

[Karlos]

Here's a little function i use ....

Code: [Select]

<?php

function secureInput($string) {
 
$string = strip_tags($string);
$string = htmlspecialchars($string);
$string = trim($string);
$string = stripslashes($string);
$string = mysql_real_escape_string($string);

return $string;
}
?>


Sorry this may seem pointless now, but this caught me eye.

I would in use two of them on input, depends on the situation. Here's my function which I use:
public function doEscape($Var) {
 if (get_magic_quotes_gpc()) {
  $Var = stripslashes($Var);
 }
 return mysql_real_escape_string($Var, $LinkId);
}

No strip_tags(); - Never used it, don't think I will.
No htmlspecialchars(); -think charsets and collations 
And trim(); - I guess you could use it, see no reason why not... It only cuts white space off the beginning and end of a string.