How could I make this PHP $_POST more secure? -Or is it secure already?

Question

On most my $_post data inputted on my site I use the following php:

$example = $_POST['textfield'];
$example = strip_tags($example);
$example = mysql_real_escape_string($example);

And then I would interact with the MySQL database...

Is this 'secure' / 'safe'?

Any major exploits to the above code?

-How could I make it secure?

Thanks alot.

Answer 1

I recommend getting yourself a simple, simple function that uses PDO and parameterizing of queries by default behind the scenes.

Here are sample use cases for query functions that use PDO in the background that I use all the time:

$iterable_resultset = query('select * from users where email = :email and username ~* :username or user_id = :user_id', array(':email'=>$email, ':username'=>$username, ':user_id'=>(int)$user_id));
// Results a resultset that is able to be foreached over.

$multidimensional_array = query_array('select * from users where email = :email and username ~* :username or user_id = :user_id', array(':email'=>$email, ':username'=>$username, ':user_id'=>(int)$user_id));
// Results in a multidimensional array with each subarray corresponding to a database row, or null.


$row_of_data = query_row('select * from users limit 1');
//Results in a single dimensional array or null.

$item = query_item("select max(score) from players where username ~* :username", array(':username'=>$username));
// Returns a single piece of information, or null.

There's a lot of advantage from a refactoring viewpoint to having simple function-based parameterizing of sql queries. PDO is often verbose, and sometimes when you're securing bad code you need simple, terse functions to fit in the place of old mysql_query() calls and the like.

In your specific case, I think that you are making the mistake of escaping before it's time, which can often introduce weird bugs.

$untrusted_input = @$_REQUEST['password'];

is all that you should do when getting the input. Don't strip tags, don't do anything until it comes time to -use- that untrusted input in sql or in html:

• In the sql, parameterize it while using the sql, filter/modify the data based on the circumstances.
• When outputting to html, escape it then, e.g. via .htmlentities($example). or using a templating engine.

Here's an example of your code turning a non-existent get/post variable (that would normally result in null) into an empty string, for just the first of many subtle things that happen when you escape too early: http://ideone.com/r7bSr

In general, when you're first getting used to the security issues that php can throw your way, I really recommend using a template engine (e.g. smarty) to help you keep your business logic and your display templates separate. A lot of people will argue about whether you need a templating engine with php because php is a templateable system already. Engine or not doesn't matter, but a template-based approach to separate your manipulation logic from your display logic is very necessary, which most programmers would probably agree on. And personally, I found it easier to learn how that separation benefits php when I was first starting out by just learning how a templating engine does things, before I could understand how to apply those principles to native php without a templating engine.

TL;DR; - Set up a templating engine, it'll get you using best practices faster.

Answer 2

It doesn't seem like there are any security issues with your code. But like grossvogel said, you shouldn't try to implement security by constantly writing that function every time you have a query, because maybe you'll forget. What I would recommend is that you write a helper function that automatically escapes the input for you. That or you could use prepared statements.

Answer 3

Apart from what has already been mentioned, a crucial part you're missing is a magic_quotes check and taking according action, this means reversing all its effects. (unless you do that at the beginning after your script starts).

If enabled, magic_quotes will escape all quotes with a backslash, and each backslash with a backslash.

Read more about magic_quotes here.