15

At the moment, I apply a 'throw everything at the wall and see what sticks' method of stopping the aforementioned issues. Below is the function I have cobbled together:

function madSafety($string)
{

$string = mysql_real_escape_string($string);
$string = stripslashes($string);
$string = strip_tags($string);
return $string;

}

However, I am convinced that there is a better way to do this. I am using FILTER_ SANITIZE_STRING and this doesn't appear to to totally secure.

I guess I am asking, which methods do you guys employ and how successful are they? Thanks

Assaf Lavie
  • 73,079
  • 34
  • 148
  • 203
Drew
  • 3,194
  • 8
  • 40
  • 62
  • dupe: http://stackoverflow.com/questions/129677/whats-the-best-method-for-sanitizing-user-input-with-php – troelskn Feb 20 '09 at 10:54
  • This approach is wrong on so many aspects! I've posted an answer below and rated each of the responses -1 for their authors refuse to think. – Nikola Petkanski Nov 01 '13 at 13:05

4 Answers4

36

Just doing a lot of stuff that you don't really understand, is not going to help you. You need to understand what injection attacks are and exactly how and where you should do what.

In bullet points:

  • Disable magic quotes. They are an inadequate solution, and they confuse matters.
  • Never embed strings directly in SQL. Use bound parameters, or escape (using mysql_real_escape_string).
  • Don't unescape (eg. stripslashes) when you retrieve data from the database.
  • When you embed strings in html (Eg. when you echo), you should default to escape the string (Using htmlentities with ENT_QUOTES).
  • If you need to embed html-strings in html, you must consider the source of the string. If it's untrusted, you should pipe it through a filter. strip_tags is in theory what you should use, but it's flawed; Use HtmlPurifier instead.

See also: What's the best method for sanitizing user input with PHP?

Community
  • 1
  • 1
troelskn
  • 115,121
  • 27
  • 131
  • 155
10

The best way against SQL injection is to bind variables, rather then "injecting" them into string. http://www.php.net/manual/en/mysqli-stmt.bind-param.php

vartec
  • 131,205
  • 36
  • 218
  • 244
3

Don’t! Using mysql_real_escape_string is enough to protect you against SQL injection and the stropslashes you are doing after makes you vulnerable to SQL injection. If you really want it, put it before as in:

function madSafety($string)
{
    $string = stripslashes($string);
    $string = strip_tags($string);
    $string = mysql_real_escape_string($string);
    return $string;
}

stripslashes is not really useful if you are doing mysql_real_escape_string.

strip_tags protects against HTML/XML injection, not SQL.

The important thing to note is that you should escape your strings differently depending on the imediate use you have for it.

When you are doing MYSQL requests use mysql_real_escape_string. When you are outputing web pages use htmlentities. To build web links use urlencode…

As vartec noted, if you can use placeholders by all means do it.

kmkaplan
  • 18,655
  • 4
  • 51
  • 65
  • 5
    Actually, mysql_real_escape_string is not entirely safe either. See http://ilia.ws/archives/103-mysql_real_escape_string-versus-Prepared-Statements.html – Michael Borgwardt Feb 25 '09 at 10:25
1

This topic is so wrong!

You should NOT filter the input of the user! It is information that has been entered by him. What are you going to do if I want my password be like: '"'>s3cr3t<script>alert()</script>

Filter the characters and leave me with a changed password, so I cannot even succeed in my first login? This is bad.

The proper solution is to use prepared statements or mysql_real_escape_string() to avoid sql injections and use context-aware escaping of the characters to avoid your html code being messed up.

Let me remind you that the web is only one of the ways you can represent the information entered by the user. Would you accept such stripping if some desktop software do it? I hope your answer is NO and you would understand why this is not the right way.

Note that in different context different characters has to be escaped. For example, if you need to display the user first name as a tooltip, you will use something like:

<span title="{$user->firstName}">{$user->firstName}</span>

However, if the user has set his first name to be like '"><script>window.document.location.href="http://google.com"</script> what are you gonna do? Strip the quotes? This would be so wrong! Instead of doing this non-sense, consider escaping the quotes while rendering the data, not while persisting it!

Another context you should consider is while rendering the value itself. Consider the previously used html code and imagine the user first name be like <textarea>. This would wrap all html code that follows into this textarea element, thus breaking up the whole page.

Yet again - consider escaping the data depending on the context you are using it in!

P.S Not really sure how to react on those negative votes. Are you, people, actually reading my reply?

Nikola Petkanski
  • 4,724
  • 1
  • 33
  • 41
  • Instead of having my comment deleted, please try to understand what I was saying. If a user had a username with tags like that I wouldn't strip the tags, I would reject the username entirely and force them to come up with a username that's less, forgive me, stupid. Passwords would be different, sure, but usernames with ` – Muhammad Abdul-Rahim Jul 10 '15 at 15:02
  • I have not deleted your comment. I flagged it and someone decided the flag was appropriate. What the user inputs is none of my business, I take care for my application to function properly. Making sense of data and escaping the data are two radically different things. Please, don't have the two mixed up. You can have whatever you want as a username, nor the presentation layer (html), nor the persistence layer (mysql, postgre, etc) have to stand in your way. Again, the question is how to escape the data, not how to filter or apply censorship to it. – Nikola Petkanski Jul 10 '15 at 19:23