Jump to content
xisto Community
jipman

Secure Php Coding tips and hints for more secure PHPing

Recommended Posts

Secure PHP coding

Today, PHP is a very common and very popular scripting language that is used by many people over the world. However, many php scripts that they make are vulnerable to 'hacks' by leaving some security holes open. This article will explain how someone can abuse your script and can alter your site/files, but also (even more important), this article will tell you how to PREVENT your site from being hacked and how to spot and fix those security holes.

Contents:

- Chapter 1 : To serve or not to serve
- Chapter 2 : MySql, friend or foe?
- Conclusion

Chapter 1

As many people know, you can use the include command to will save you from doing tedious copy's and paste's by including a file directly into the script for processing. Almost everyone that uses PHP in their website use it to make it easier to serve it's contents.

a small example




INDEX.PHP------------<?php  $page = $_GET['page'];   include ($page.'txt');?>

The include-line opens the file $page, add the extension .txt to it and virtually pastes the contents instead of the include line. variable $page is filled by using a GET request in your browser.

e.g. <a href="index.php?page=foobar">Click here</a>

However, what if someone does this

index.php?page=http://forums.xisto.com/no_longer_exists/

This will have YOUR index.php running PHP code (from the file exploitcode.txt , the script adds the txt extension itself (in this case)) from another site, there's no need to say that now you're entire site is open and the attacker can run any code he/she wants at will.

We ofcourse certainly don't want this, so here are a few ways to prevent this from happening.

1. using the file_exists command that verifies if a file exists on the server itself. eg.
 



<?php  $page = $_GET['page'];  if (file_exists($page)) {    include ($page.'txt');  }?>

This will first check if the file exists on the server itself and will not include files from other servers. (In php5 it's slightly different, see http://php.net/manual/en/function.file-exists.php)

2. Second method (my personal favorite), i like to call this method barrier style. It's perfect only it needs quite more code. example:
 



<?php  $page = $_get['page'];  switch ($page)    default:      include ('home.txt');      break;    case 'foobar':      include ('foobar.txt');      break;    case 'links':      [I]etc etc[/I]

This actually places some sort of barrier between the user input and the execution. This is what is does.

the switch is an extended if-then sequence, it basicly checks every 'case' and watches if there is a value stated behind it that matches the input variable. If it matches it then will do the action stated under it and jump out of the switch at the break command. Since the input is always checked so it's no use to enter something that will leave your script open, e.g. If you'd enter http://forums.xisto.com/no_longer_exists/, it would not match up with any of the cases and will force the default action to be executed.

3. Of course there are many other ways to do this but the most important thing is to check the user input.

This brings us to the second chapter, mysql

Chapter 2

It's also a common sign for site use databases like MySQL, since I don't give a ** about M$-SQL, I will discuss MySQL only.

For password authentications, MySQL databases can prove to be very usefull and hold a few advantages to flat-files, they are encrypted, they are password protected and they are way easier to manage. Here's a little example system
 



<?php  $handle = mysql_connect($server,$user,$pass);   mysql_select_db($databasename);       $input_user = $_POST['user'];   $input_pass = $_POST['pass'];   $result = mysql_query('SELECT * FROM users WHERE user = '{$input_user}' AND pass = '{$input_pass});   if (mysql_num_rows($result) == 0)   {      echo 'Not logged in';    }    else    {      echo 'Logged in';     }?>

The system first connects to the database server with the username and password. Then it selects the database. (note. I left out the error handling code because it's not relevant in this case).

Then it searches the table users for records (rows) that have $user as user and $pass as password. Since every user must be unique, all you need is to count the number of rows that has the correct password/username. For that we use the mysql_num_rows command, it simply returns the number of rows that are in the result of the previous query.

This system can also be easily exploited.

let's say that we have something like this

SELECT * FROM users WHERE user = 'foobar'-- AND 'a'='a' AND pass = 'thisdoesnotmatter'

In this case, the inputted username is foobar'-- AND 'a'='a
Since the input is not checked, the script plainly passes the input to the query. The query will do something different now, the -- tells the SQL server to ignore everything that comes after it so the query would look like this : SELECT * FROM users WHERE user = 'foobar' ...... Well I guess anyone would see this is a free login without even needing to know the password. There are endless variations like using .. OR .. statements, or UNION statements etc etc.

To prevent this kind of abuse you need to know the following stuff. MySQL is very sensitive for quotes placed on the wrong place. Luckily, there is a command that will addslashes to a string to neutralize those bloody quotes, mysql_real_escape_string() does that, it makes the input foobar'-- AND 'a'='a looks like
foobar\'-- AND \'a\'=\'a, which can be inserted into a query without a prob since it will check for the username foobar'-- AND 'a'='a, which is not a danger. Because now it cannot cut off the rest of query as it previously could.

Conclusion

This kinda wraps it up for today, these are the most important things to look at if you ever decide to create some site in PHP. There is one rule that is highly important and if you live by it you should be quite safe:

Don't ever ever trust the bloody user input

Why? you ask, that's logical, it's USER input.

normal-user = user
american = user
hacker = user

On the other hand, if you do get hacked, check the serverlogs and see how he got in. These mistakes aren't beginner-mistakes, most bulletin board software system exploits also work in this way (slightly more complicated though).

tip: If you learn to write cleanish and neat code, indents and stuff. It's much easier to debug :(.

There could be errors in this, if you see any, feel free to reply below
version 0.1


Share this post


Link to post
Share on other sites

SELECT * FROM users WHERE user = 'foobar'-- AND 'a'='a' AND pass = 'thisdoesnotmatter'That is the example of free entry into the system.I just have one question. AND requires that both pieces be true so: SELECT * FROM users WHERE user = 'foobar' AND 'a'='a' AND pass = 'thisdoesnotmatter' should return empty unless there is a use with name foobar and password thisdoesnotmatter.I'm assuming that the -- after user = 'foobar' changes this so access is granted,but what exactly does -- do? would it change the result form 0 (false) to -1 (because it is the decrement symbol)?~Viz

Share this post


Link to post
Share on other sites

no, it's a just chops off the rest of the query. eg

SELECT * FROM users WHERE user = 'foobar' -- AND 'a'='a' AND pass = 'thisdoesnotmatter'

will become

SELECT * FROM users WHERE user = 'foobar'

I should have explained it better but the -- makes everything behind it comment so the server ignores them. It's not really pretty I guess, but it works.

Read about them here:
http://forums.xisto.com/no_longer_exists/

Share this post


Link to post
Share on other sites

So what that does is makes it so that you still need to know the username? Am I right on that?As opposed to entering:username: a' OR 1 OR 'apassword: a' OR 1 OR 'awhich should result in:SELECT * FROM users WHERE user = 'a' OR 1 OR 'a' AND pass = 'a' OR 1 OR 'a'which should (if I did the logic in the correct order) return a successful entry no matter what.Tell me if I did this correctly please (trying to understand how the hacker could get in to better keep him/her/it/they/we/whatever out).~Viz

Share this post


Link to post
Share on other sites

yup, above situation is incase you only know the usernameIf you don't know the username and the password, what you said with those OR's probably would work too but is quite difficult to understand, and i also recall something about ... OR .. OR .. structures. Let's make it a bit easierusername: a' OR '1password: a' OR '1SELECT * FROM users WHERE user = 'a' OR '1' AND pass = 'a' OR '1'although you should not put numeric values between quotes, you will get away with this. Since the INTEGER 1 still equals the string 1 (same byte)

Share this post


Link to post
Share on other sites

Well... in the ascii chart there's only one '1', or do I miss something here ? Anyway, if you would make this comparison in visual basic, it would automatically compare the integer 1 to the real value of the string 1, which does make 1 = 1.

Share this post


Link to post
Share on other sites

I understand that they are compared equal by the program. But you said they were same byte. And yes, there is only one ASCII '1', it is hex code 31. But integers are stored in plain old binary, not BCD (binary coded decimal) so the number 1 (not the string) is in hex 01. Thus not the same byte, although higher level programs compare them equal.~Viz

Share this post


Link to post
Share on other sites

Just a correction!

<?php  $page = $_GET['page'];  if (file_exists($page)) { 	include ($page.'txt');  }?>
this will result in including the file (when $page is "home") hometxt instead of home.txt! (spot the dot!)It should be:
<?php  $page = $_GET['page'];  if (file_exists($page.".txt")) { 	include ($page.".txt");  }?>

That's what I have learned, but maybe what you did would work too.

I kinda feel bad about includes that allow to include external scripts/files! It isn't right to allow such things, yes, perhaps there are some webpages that use this but it's a BIG security flaw, i think. It makes it possible to use someone else's user database or MySQL data. And it could be possible to load the source of for example "config.php" where the MySQL password / username could be stored. Well it's my point of view, and i think it should be made to load only local files!

The switch default is the best solution indeed, and even the switch alone. There's no way to get around these things.
At least haven't found it yet :)

Bakr_2k5

EDIT:
$query = "SELECT * FROM menus ORDER BY id ASC";$menus_result = mysql_query($query);while( $menus=mysql_fetch_array($menus_result) ) {  switch($_GET['page']) {	case $menus['name']:	  include($menus['name'].".php");	  $menu_found = TRUE;	  break;  }}if( $menu_found != TRUE ) {  echo "Page not found!";}

This is my current page load script. It loads the menu items from MySQL for some dynamically way. But couldn't use the switch default thing because it would load the default page every time case didn't match $menus['name'], so i put the menu_found = TRUE and a check in it to work as a default handle.
It basically does the same thing as default, if I'm right.

Thought it could be handy for someone that needs it :D
Edited by bakr_2k5 (see edit history)

Share this post


Link to post
Share on other sites

just checked the MySQL hack in a script that i just created.....for some reason it doesnt actually work. my script does what it was designed to do and block entry but i cant understand why...here is a very simplified version of the script......if -- did kill the rest of the query it should technically be able to hack this script but it doesnt seem to workryan<?PHP$user=$_POST['user'];$pass=$_POST['pass'];if(!$user||!$pass){......error message}else{$db=mysql_connect('*******','******','****');$sel=mysql_select_db("*****");$enc=md5($pass);$query="SELECT * FROM `users` WHERE `user` = '$user' && `pass` = '$enc'";$do_query=mysql_query($query);$num=@mysql_num_rows($do_query);if($num!="1"){.......login fail}else{session_start();$_SESSION["user"]=$user;header("Location: test.php");}}?>***************EDIT*********************$querya="SELECT * FROM `users` WHERE `user` = '$user'";$do_querya=mysql_query($querya);$numa=@mysql_num_rows($do_query);if($numa!="1"){.....no user}else{$query="SELECT * FROM `users` WHERE `user` = '$user' && `pass` = '$enc'";$do_query=mysql_query($query);$num=@mysql_num_rows($do_query);if($num!="1"){.....password wrong}else{session_start();$_SESSION["user"]=$user;header("Location: test.php");}}this would fix that particular hack even if the user input wasnt checked if im not mistaken

Edited by ryanlund2323 (see edit history)

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×
×
  • Create New...

Important Information

Terms of Use | Privacy Policy | Guidelines | We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.