I recently asked a question on Stackoverflow
I was a bit confused over a concept in programming/web-development I had never seen before, which turned out to be "Abstraction". I was provided with some good answers.
Several people mentioned that my code didn't look like it was checked for errors pretty well. The post has some long answers and my original one got pretty long too, so I decided to elaborate here.
I should note that my native language is Dutch. Most variable names will be in dutch, though some resemble their English counter-part. Though I'll be re-writting some of it in English randomly.
register.php
The HTML of this page contains a hidden field with the name 'postback'.
<input type="hidden" name="postback" value="" />
<input type="submit" name="btnOk" id="btnOk" value="Register" />
<input type="reset" name="reset" value="Reset" />
In the source code, you'll find this:
if(isset($_REQUEST['postback'])) {
$allOk = true;
...This is where the formchecking begins. What we were taught to do, let's leave whether this is a good or bad practice out of it for a second, is to control the input of every field for possible SQL injection, XSS and other stuff.
The basic idea is that the moment something bad is detected, the allOk variable is set to
false and can never be set to
true again.
Here are a couple of examples of SQL injections we got from our instructor:
admin'--
' OR 1=1--
' UNION SELECT 1, 'hackerX', 'letmein', 1--
' HAVING 1=1--
' GROUP BY users.id HAVING 1=1--
' GROUP BY users.id, users.login HAVING 1=1--
' UNION SELECT SUM(login) FROM users--
'; INSERT INTO users VALUES(666, 'hacker', 'letmein', 0xffff)--
' UNION SELECT @@version,'1','1','1'--
' UNION SELECT MIN(login),1,1,1 FROM users WHERE login>'a'--
' UNION SELECT MIN(login),1,1,1 FROM users WHERE login>'admin'--
' UNION SELECT password,1,1,1 FROM users WHERE login='admin'--
So how do I check my forms? The way I remember doing it most. And this is how:
Step 1
Using $_POST[], I make all the variables I need.
Example:
$naam = addPostSlashes($_POST['naam']);
$straat = addPostSlashes($_POST['straat']);
$paswoord = addPostSlashes($_POST['pasw']);
...
addPostSlashes() is a function that checks if the php.ini file has magic quotes enabled or not. If not it applies the addslashes() method and in the other case it's been done already.
I saw people mentioning the mysql_real_escape_string() function. I looked it up on PHP.net and compared it to the addslashes() method. The documentation itself doesn't show a lot of difference and some debugging didn't either, though I haven't tried the SQL injections yet. I doubt they'd have created mysql_real_escape_string() if it does the exact same thing as an existing function.
Part 2 - Regular Expressions
I use a couple of regular expressions to test the variables.
$rexSafety = '/^[^<,"@$?=>|;#]+$/i';
$rexMail = '/^[a-z0-9\._]+@([a-z0-9-_]+\.)+[a-z]{2,3}$/i';
I'd like to mention that the Regular Expressions you see here have been tested and work. The only tiny problem I'm having is with the first one, which doesn't seem to allow empty strings. Which can be a pain for non-required fields in a registration form.
The first one is used as follows:
if(!preg_match($rexSafety,stripslashes($naam))){
$allOk = false;
$naamError = ' please don\'t use < , @ # - > $ ; = | or "';
}I'm checking if any of these characters are present in the received string and if there is even one, the entire registration gets canceled. Next to the field where the error was found appears a message explaining what went wrong to the user. In this case, asking to please not use any of the following characters.
Rest assured, a potential customer accidentally filling in something wrong won't have to re-write the whole thing. The page is reloaded not only with the explanations, but also has all the previously entered text in the correct place, with the exception of passwords not being reloaded.
Thinking about this, I realize that anyone with a @ in their password is gonna be pretty miffed at me for doing this. As far as I'm aware, you can't escape a @ or a # sign and they can both be used in SQL injection. Please correct me if I'm wrong. And should you do so, please refrain from such comments as: "what a horrible piece of coding" and other such comments. I'm here to learn, not to get insulted.
Part 3 - All Ok!
Once everything is checked and no errors are found, I call upon the SQL class and the User class. They respectively represent the SQL actions I want to perform and the actions on the User I wish to perform. I should perhaps mention that the register.php file has a
require statement for both classes at the top of the file.
So this happens:
$sql = new SqlObject();
$newUser = new User('',$naam,sha1($passw),...);
$sql->addUser($newUser);
All of those are escaped. Just have to remember it's all stored with escaped characters, as to make sure logging in works.
Let's look at an example of what the User.class.php file looks like:
<?php
class User {
// members
var $id;
var $name;
var $password;
// constructor
function User($id=-1,$name='',$password='') {
$this->id = $id;
$this->name = $name;
$this->password = $password;
}
}
?>
This is shortened, but you get the general idea I hope.
The SQL class has
require "User.class.php";
written on the first line, which enables it to use instances of the User class.
Which brings us to the addUser() function:
function addUser($user){
$insQuery = 'INSERT INTO users(naam,paswoord,...)';
$insQuery.= " VALUES ('".$user->naam."', '".$user->paswoord."')";
@mysql_query($insQuery) or showError("user insert failed");
}I highly doubt this but I'll mention it anyway: the ... are meant to indicate more fields, not actual input.
So that's how we've been taught to do it. That is to say, we were shown the basics and were allowed to choose our own way of implementing it.
I read the term "prepared statement" a few times. I looked it up and it looks interesting enough.
To answer Mr. Vinko's example: the string
John O'Donell will be entered into the database as
John O\'Donell, that is true. Your concern seems to be that the escaping of the characters should happen at the insertion, and not during the checking for SQL injections(and similar stuff). To be honest, I've never considered doing this, because of the simple reason that it's not the way I did it the first time I did this type of formchecking. The fact that it worked, was enough for me untill now. This whole ordeal has gotten me thinking, and I'm beginning to believe I'm mixing up a few things.
EDIT:
I read all the things most of you said and it's starting to make sense. I can divide it all into 2 basic steps now
- Check for faulty input/possible injections without escaping anything
- If everything checks out, send it to the PHP that handle database insertion and escape it there.
That's what I think this all comes down to. I hope I got it right this time...
I hope this wasn't too long and I realise this was more of an explanation to certain people than a proper blog entry, but I did what I said I'd be doing when I made my first post on this blog: learning. If anyone has any pointers or tips on the subject of formchecking or prepared statements, feel free to tell me.
Though if you choose to do so, please don't say stuff like "That's a bad practice, the way I do it is better!" and the walk off without even offering an example. If you're gonna say I'm wrong or it could be better, then take the time to prove it. Offer me a good reasoning and I will listen/read. I'm not the kind of guy who sticks to what he believes no matter what for fear of new things, I just hope the rest of you out there aren't either.
