views:

757

answers:

6

Question: Is preventing XSS (cross-site scripting) as simple using strip_tags on any saved input fields and running htmlspecialchars on any displayed output ... and preventing SQL Injection by using PHP PDO prepared statements?

Here's an example:

// INPUT: Input a persons favorite color and save to database
// this should prevent SQL injection ( by using prepared statement)
// and help prevent XSS  (by using strip_tags)
$sql = 'INSERT INTO TABLE favorite (person_name, color) VALUES (?,?)';
$sth = $conn->prepare($sql);
$sth->execute(array(strip_tags($_POST['person_name']), strip_tags($_POST['color'])));


// OUTPUT: Output a persons favorite color from the database
// this should prevent XSS (by using htmlspecialchars) when displaying
$sql = 'SELECT color FROM favorite WHERE person_name = ?';
$sth = $conn->prepare($sql);
$sth->execute(array(strip_tags($_POST['person_name'])));
$sth->setFetchMode(PDO::FETCH_BOTH);
while($color = $sth->fetch()){
  echo htmlspecialchars($color, ENT_QUOTES, 'UTF-8');
}
+4  A: 

It's even more simple. Just htmlspecialchars() on user-controlled input is enough. The strip_tags() is only useful if you already want to sanitize data prior to processing/save in database, which is often not used in real world. HTML code doesn't harm in PHP source, but PHP code may do so if you use eval() on non-sanitized user-controlled input or that kind of evil stuff.

This however doesn't save you from SQL injections, but that's another story.

Update: to get clean user input from the request to avoid magic quotes in user-controlled input, you can use the following function:

function get_string($array, $index, $default = null) {
    if (isset($array[$index]) && strlen($value = trim($array[$index])) > 0) {
         return get_magic_quotes_gpc() ?  stripslashes($value) : $value;
    } else {
         return $default;
    }
}

which can be used as:

$username = get_string($_POST, "username");
$password = get_string($_POST, "password");

(you can do simliar for get_number, get_boolean, get_array, etc)

To prepare the SQL query to avoid SQL injections, do:

$sql = sprintf(
    "SELECT id FROM user WHERE username = '%s' AND password = MD5('%s')",
        mysql_real_escape_string($user),
        mysql_real_escape_string($password)
); 

To display user-controlled input to avoid XSS, do:

echo htmlspecialchars($data);
BalusC
How can I modify to also protect me from SQL injections in addition to XSS?
TimTim
Use `mysql_real_esacpe_string()`: http://php.net/manual/en/function.mysql-real-escape-string.php. Furthermore, you also need to take `get_magic_quotes_gpc()` into consideration. If this returns true on the PHP environment, you need to run `stripslashes()` on the input as well.
BalusC
If I used PDO prepared statements, does that prevent SQL injection?
TimTim
Oh, they certainly do, if used the right way.
BalusC
@BalusC, will my updated original post protect me from all malicious activity from users (XSS, SQL injection and anything else I might not know about)?
TimTim
Not on *input* .. on *output*. That's the whole point, that everybody keeps missing.
troelskn
@troelskn, will you post an answer than that informs me how to properly protect my web-application from both SQL injection and XSS
TimTim
@TimTim: if you get rid of `strip_tags()`, it may be OK (if magic quotes is *always* turned off regardless of the environment you run on). @troelskn: either I don't understand you or you don't understand us. Note that there was an **update** as per the comments.
BalusC
@BalusC, what's wrong with using strip_tags() ? Or is that just adding even extra security that's unneeded?
TimTim
@TimTim: it's unneeded and doesn't avoid HTML injections. `htmlspecialchars()` do both XSS prevention (escaping the `<` and `>`) and HTML injection prevention (escaping the `"` and `'`). Also, in most circumstances you'd like to save the user input as unchanged as possible, so that you know what the user *actually* inputted, so that you can eventually take social actions.
BalusC
@BalusC, I only want the user to input plain text ... as such, in this case strig_tags would be of benefit then right. And also, to confirm - my example in the original post would protect me from both XSS and SQL injection ... correct (or does it need modifying?)
TimTim
The `strip_tags()` is not needed unless you want to have something like as `if speed > 100` stored as `if speed 100`. On the other hand.. Just try to do XSS / SQLi attacks on your webapp yourself..? :)
BalusC
+2  A: 

strip_tags is not necessary. In most cases strip_tags is just irritating, because some of your users may want to use < and > in their texts. Just use htmlspecialchars (or htmlentities if you prefer) before you echo the texts to the browser.

(Don't forget mysql_real_esacpe_string before you insert anything into your database!)

Emil Vikström
+1  A: 

Simple answer : no

Longer answer : There are ways to inject xss that PHP strip_stags cannot avoid.

For better protection try HTML purifier

MaxiWheat
Thank's for the tip! I don't think it's the only solution to this exact problem, but it's cetainly good if I want to allow some HTML formatting.
Emil Vikström
Let's imagine we have a page with only `<?echo strip_tags($_GET['query']);?>` in its body. What injections are possible here?
naivists
.. when `strip_tags()` is used *without* `htmlspecialchars()`.
BalusC
Can XSS be achieved even if htmlspecialchars is used on output?
TimTim
@navist, please note - what you wrote is not my use case. I was talking about using strip_tags on INPUTTED data, not OUTPUTTED (displayed) data
TimTim
@MaxiWheat, Can XSS be achieved even if htmlspecialchars is used on output?
TimTim
@TimTim : I don't think so since every html tag is escaped and diplayed as is so no style or any kind of event can be triggered
MaxiWheat
A: 

It depends on where and how you want to use the user data. You need to know the context you want to insert your data in and the meta characters of that context.

If you just want to allow the user to put text up on your website, htmlspecialchars suffices to escape the HTML meta characters. But if you want to allow certain HTML or want to embed user data in existing HTML elements (like a URL into a A/IMG element), htmlspecialchars is not enough as you’re not in the HTML context anymore but in the URL context.

So entering <script>alert("xss")</script> into a image URL field will yield:

<img src="&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt" />

But entering javascript:alert("xss") will succeed:

<img src="javascript:alert(&quot;xss&quot;)" />

Here you should take a look at the fabulous XSS (Cross Site Scripting) Cheat Sheet to see what contexts your user data can be injected in.

Gumbo
@Gumbo, does my updated original post protect me from your example of not using a script tag?
TimTim
@TimTim: It just depends on the context where you want to use that data.
Gumbo
A: 

The general rule/meme is "Filter Input, Escape Output." Using strip_tags on your input to remove any HTML is a good idea for input filtering, but you should be as strict as possible in what input you allow. For example, if an input parameter is only supposed to be an integer, only accept numeric input and always convert it to an integer before doing anything with it. A well-vetted input filtering library is going to help you a lot here; one that isn't specific to a particular framework is Inspekt (which I wrote, so I'm a bit biased).

For output, htmlspecialchars should be able to escape XSS attacks, but only if you pass the correct parameters. You must pass the quote escaping style and a charset.

In general, this should remove XSS attacks:

$safer_str = htmlspecialchars($unsafe_str, ENT_QUOTES, 'UTF-8');

Without passing ENT_QUOTES as the second parameter, single-quote chars are not encoded. Additionally, XSS attacks have been demonstrated when the correct charset is not passed (typically UTF-8 will be adequate). htmlspecialchars should always be called with ENT_QUOTES and a charset parameter.

Note that PHP 5.2.12 contains a fix for a multibyte XSS attack.

You may find the OWASP ESAPI PHP port interesting and useful, although the PHP version is not complete AFAIK.

Funkatron
@Funkatron, so is my updated original post protect me from all evil?
TimTim
It looks to be relatively safe, yes.
Funkatron
+1  A: 

Yes, using PDO prepared statements protects from SQL injection. The SQL injection attack is based on the fact that the data submitted by the attacker is treated as a part of the query. For example, the attacker submits the string "a' or 'a'='a" as his password. Instead of the whole string being compared to the passwords in the database, it is included in the query, so the query becomes "SELECT * FROM users WHERE login='joe' AND password='a' or 'a'='a'". The part of attacker input is interpreted as a part of the query. However in case of prepared statements, you are telling the SQL engine specifically, what part is the query, and what part is data (by setting the parameters), so no such confusion is possible.

No, using strip_tags will not always protect you from cross-site scripting. Consider the following example. Let's say your page contains:

<script>
location.href='newpage_<?php echo strip_tags($_GET['language']); ?>.html';
</script>

The attacker submits the request with "language" set to "';somethingevil();'" . strip_tags() returns this data as is (there are no tags in it). The produced page code becomes:

<script>
location.href='newpage_';somethingevil();'.html';
</script>

somethingevil() gets executed. Replace somethingevil() with actual XSS exploit code.

Your last example with htmlspecialchars() will protect against this one, because it will escape single quotes. However I have seen even weirder cases of user-supplied data inside JavaScript code, where it is not even within a quoted string. I think it was in the variable or function name. In that last case no amount of escaping will probably help. I beleive that it is best to avoid using user input to generate JavaScript code.

Alla