Top PHP Security Hole

While this may not be the most dangerous security hole that can exist in PHP code, it is certainly the most common and is dangerous enough - opening up the code to at least the potential of injection attacks and almost certainly allowing the data to be filled with junk.

$field = $_POST['field'];

I have lost count of the number of pieces of PHP that I have seen that contain statements that look like that (where 'field' can be any field name).

Such statements are completely pointless as all that they do is to copy the value from one field to another where in fact they could have simply substituted the $_POST[] variable in each spot where they wanted to reference the field instead of copying it to a second field and referencing that instead. The number of characters this saves is relatively small particularly considering that the potential damage is huge.

In fact NOT doing this copy makes the security level of the subsequent statements far more obvious.

Any field in a program that can potentially contain any value at all is known as a 'tainted' field. The field itself is considered tainted regardless of what it contains because you can't tell from just looking at a small section of code whether the content is valid or junk. PHP helps us to distinguish tainted fields from untainted ones by naming many of the tainted ones with names starting $_ to identify that the values have come from an external source. The reason the register globals setting was done away with was that it introduced an even bigger security hole than the one we are discussing by making all of the variables in the code tainted.

Assigning a value from $_POST to another variable name makes this new variable tainted as well but without the PHP supplied naming convention that distinguishes tainted from untainted fields. So now we need to remember that $field is tainted and can potentially contain anything. At least by using the original field name in the subsequent code it is more obvious where you can be inserting junk into your database or HTML and where the code might be open to injection attacks. Copying it and using a field name that you have deliberately tainted but where that isn't obvious from the field name simply hides many of these potential problems and requires that you examine the entire code thoroughly in order to work out whether a field is tainted or not.

Simply copying a value from one field name to another serves no purpose whatever so the problems with doing so in this case mean that there is absolutely no reason whatever for doing so. So why do so many people do it in their code? Well a lot of PHP books (as with almost all computer books) leave out a lot of code from their example scripts in order to reduce the code to just the essentials of the code construct they are currently demonstrating. The part of the code that they can leave out and still have the example work - because only the students are going to run the code - is the 80% of any program that handles field validation and error handling. These assignment statements represent that 80% of the code that has been left out.

Unfortunately some books forget to say that these statements simply represent all of the missing code and even where they do, many students get so used to seeing these statements that they forget that they are supposed to replace them with the 80% of the complete script that handles the validation and error handling. If books were to make this clearer then there would be far fewer scripts written that are missing most of the necessary code and which are therefore definitely open to massive quantities of junk data even when the person has applied other (often inappropriate) code to attempt to prevent the injection attacks that could never occur had the validation and error handling been included.

Perhaps PHP books and online courses could make it more obvious that there is a huge chunk of the code needed for live sites that has been omitted by replacing these simple assignment statements with the following that at least makes it clear that code has been omitted.

function validate($fld) {
// add code to actually validate or at least sanitize the field
return $fld;
$field = validate($_POST['field']);

At least if the statements were coded this way in the code examples it would be immediately obvious that there is code that has been left out to keep the example to a manageable size - plus it only needs that dummy function in some of the examples, it could be implied in most of them once the student gets used to it being needed. Calling a validate function even one that doesn't contain any actual code would at least act as a reminder that the field needs to be validated before it gets moved to a different field name in scripts intended for others to use.

Of course in a real live script you would probably need multiple validation functions depending on just how each individual field needs to be validated and so would need to change the function names but at least with using this as your initial code you have a function call there to rename and a reminder that you actually need to do appropriate validation or sanitizing at that spot in order to move a value from a tainted field into an untainted one without tainting the field you copy it to.

That then just leaves you with deciding whether to validate or sanitize the field and what form that processing needs to take. The answer to whether to validate or sanitize is clear cut. You need to validate user inputs - $_POST and $_GET - where you can return an error message to the user telling them the value they entered is invalid and that they should try again. You should sanitize all other inputs - $_COOKIE, $_SERVER etc - since you can't return an error message in these instances but still need to check that the content is at least potentially valid (sanitizing removes any invalid characters so that if the value has been tampered with it can at least do no harm when passed to the subsequent code). As for how to carry out the validate/sanitize step, that depends on what you expect to be valid for the individual fields - you then use the most appropriate way of performing that test using built in functions or filters if such are available or creating a regular expression when no simpler option is available. The more you can rely on built in PHP code rather than writing your own for this processing the less likely there is to be errors that allow invalid values to get through.


This article written by Stephen Chapman, Felgall Pty Ltd.

go to top

FaceBook Follow
Twitter Follow