1

I would like to be nice to my users but don't want to get malicious code by mistake. Looking in SO many questions I decided to go whitelisting, so I get only characters I approve.

How is it possible to be kind to the user, so I let them enter many characters and still keep my inputs safe from malicious code?
I am using Apache with PHP 5.3.8, I haven't incorporated PDO because the time I heard of it it I was deep into the project and feared it might be too big of a change.

Any ideas would help!

Ted
  • 3,805
  • 14
  • 56
  • 98

2 Answers2

4

If it can be output to a user, you must prevent the potentially malicious user from including HTML tags in his code. For example, if, in that post, I could include a script tag, that could be very dangerous for any user reading my post. To prevent this, you want to use htmlentities:

$clean_data = htmlentities($_POST['data']);

That way, my <script> tag will be translated as &lt;script&gt; so it won't harm users when displayed by their browsers.

Now, if you want to save my post in your database, you must beware of SQL injections. Let's say you're saving my post with that query (yes, you shouldn't use mysql_* functions as they are deprecated, but that's just to explain the idea) :

mysql_query($db, "INSERT INTO posts(data) values('$clean_data');");

Sounds good ? Well, if I'm nasty, I'll try to include that post :

test'); DELETE FROM posts; SELECT * FROM posts WHERE data = '

What your MySQL gets is then

INSERT INTO posts(data) values('test'); DELETE FROM posts; SELECT * FROM posts WHERE data = '');

Ouch. So, you must basically prevent your user from including quotes and double quotes in his post, or, more precisely, you should escape them. That really depends on the library you are using, but in the obsolete one I used, that would be written :

$really_clean_data = mysql_real_escape_string($db, $clean_data);
mysql_query($db, "INSERT INTO posts(data) values('$really_clean_data');");

So, with the above malicious post, MySQL would now receive

INSERT INTO posts(data) values('test\'); DELETE FROM posts; SELECT * FROM posts WHERE data = \'');

Now, to MySQL, the whole INSERT INTO posts(data) values('test'); DELETE FROM posts; SELECT * FROM posts WHERE data = ''); part is a correct string, so what happens is what you want to happen.

Basically, that's all you need in almost all the cases. Just remember that, when you feed user data to an interpreter (it can be a web browser, a SQL engine or many other things), you should clean that data, and the best way to do it is to use the dedicated functions that should come with the library you are using.

Fabien
  • 12,486
  • 9
  • 44
  • 62
2

Just to add to the answer that Fabien already gave, text data types are not the only item you need to be concerned with. Numbers are just as important. Type casting your variables is one way to handle that. For example,

$really_clean_data = mysql_real_escape_string($db, $clean_data);
$query = "UPDATE posts SET post = '".$really_clean_data."' WHERE id = '".(int)$_POST['id']."'";

All data submitted, including the id number of the post you'll be looking to update, is just as suspect and just as open to malicious fiddling as a textarea or text input entry. When you know it should be an integer (number) you can cast it as such. If it's not numeric, that (int) will cast it as 0. You can't update row 0 so the SQL query will fail.

To check for that failure prior to executing the query, you can check the posted data to see if it is a number using the is_numeric() function.

if(!is_numeric($_POST['id'])){
    die("Post id is not a number.");
}

For input fields, most notably username and password fields, you can set a max length and check. For example,

if(strlen($_POST['username']) > 24){
    $error = "username is too long";
}

If you can get creative about it, preventing SQl injection is actually fun! Keep in mind that a year from now everything on this page could be completely outdated and irrelevant. Security is a process, not a step, so keep on top of it.

Strixy
  • 568
  • 5
  • 15