security: PHP SELF & xss

[jamie]

i only found out 2 days ago about the PHP_SELF xss flaw [seancoates.com...]

wow! i had to check every single form on our site and was actually able to hack two of them - apparently this hole has been open for several years - can't believe it slipped under my radar. ooops.

anyway, now the forms are secure. however, this has led me onto thinking about other vulnerabilities in _GET / _POST vars.

presumably the same can happen if you echo a _GET var directly in the html, e.g. within a href?

in which case, should i be filtering all _GET and _POST vars at the top of each page. e.g. a sitewide function which is called on every page:

foreach ($_GET as $k => $v) {
 $_GET[$k] = htmlentities($v, ENT_QUOTES);
}

how would this play with UTF-8 encoding though. i kind of like to store my chars as é, ä in the database, not as é, etc

would

strip_tags()

work just as effectively without encoding special chars?

would be great to know what others do.

cheers

p.s. just in case you are wondering, i am meticulous about filtering/escaping mysql input. it's just i've never applied the same methods/ideas to html output.

[coopster]

It's not necessarily the html output that is of concern here, it's the user-supplied variables (html input, if you will) that is of concern. $_GET and $_POST are user-supplied. So are some of the $_SERVER variables, including PHP_SELF.

Typically you know what you expect to be in a variable or the pattern it should resemble, or if it should be all numbers, etc. Validate before use. For outputting variable information as html, strip_tags will ensure the user isn't trying to pass html into your process and htmlspecialchars() should be enough when it is embedded in an element to encode special characters. Don't forget that you can pass htmlspecialchars() an optional charset argument for UTF8 to override the default.

[jamie]

thanks coopster

i do validate whenever a database is involved (which is nearly all of the time), i strip tags and clean email output and headers, etc, but i have also caught myself doing lazy things like:

echo <a href="?' . $_GET['id'] . '">click</a>

which is open to the attack by appending the following to the GET param in the url. e.g.:

page.php?id=45

followed by

"><script>alert('hallo')</script><foo="

i've revised most of my code now and realise it is only in very few places that i've done this, but still.. a salutory lesson!

[Matthew1980]

Hi all,

Just like coopster states, so long as you sanitise all of your incoming data you should be fine, sanitising varies with the size and scope of your project though, and I always think that this type of things should have a degree of flexibility to it from the admin angle.

Your little excerpt there should be acceptable so long as you 'encase' the $_GET like this:-

echo <a href="?'.(int)strip_tags($_GET['id']).'">click</a>

Though I wouldn't normally advocate this, as realistically you should have a set function/method of sanitising your global's directly AFTER the form has been set/given state - same goes for the URL/link's, but everyone has their own preference.

Don't forget though, that the form's default action is to post to itself, so you don't need to specify a file to use if your using the same file to post to; having said that, I always fill in the form's attribute so that when you pass strict (x)html validation - and it never hurts for you to just pop the name of the file in there for that reason..

Cheers,
MRb

[jamie]

]hi both,

i am still a bit confused between where to use strip_tags and where to use htmlspecialchars.

above coopster says

For outputting variable information as html, strip_tags will ensure the user isn't trying to pass html into your process and htmlspecialchars() should be enough when it is embedded in an element to encode special characters

what does the when it is embedded in an element mean?

2 examples: when i am echoing posted data to an <input>:

<input name="msg" value="<?php echo $_POST['msg'] ?>">

vs echoing to a <p>

<p><?php echo $_POST['msg'] ?></p>

if i understand correctly, in the first example i need to use htmlspecialchars() but in the second strip_tags() is enough?

many thanks!

[enigma1]

strip_tags will remove the html tags. There is a parameter to the function to allow specific tags. htmlspecialchars will convert quotes and angle brackets into html entities.

Every case maybe different because you may want to allow some characters from user input. If you think of bbcode like tags, it uses certain identifiers to parse input in a different way, as the form fields typically use the htmlspecialchars function. Since all content is filtered the different tags allow the code to be emitted in HTML in a controlled manner.

Use the htmlspecialchars if you want to preserve all user input in HTML entities. Use the striptags if you know angle brackets are not going to be used with the field, or if the field won't parse html in anyway.

If you do an (int) cast you don't have to do a strip_tags, besides if someone tries to inject don't expect the input to be an integer. So doing
(int)$_GET['id']
is enough.

Also you don't parse the $_GET/$_POST arrays like this:
foreach ($_GET as $k => $v) {
$_GET[$k] = htmlentities($v, ENT_QUOTES);
}
why wasting processing cycles which are subject to the unknown fields submitted (you don't sanitize the keys in that code btw).

Only the script which aware of the input details should process them and only the necessary fields. So if the script you invoke expects to see id and msg

$id = isset($_POST['id'])?(int)$_POST['id']):0;
$msg = (isset($_POST['msg']) && is_string($_POST['msg']))?htmlspecialchars($_POST['msg']):'';

This code does a basic cleanup of the 2 variables only. It doesn't care if the input contains 100 variables it only checks for 2. By type and by value.

[rocknbil]

just in case you are wondering, i am meticulous about filtering/escaping mysql input.

Don't forget that there are many other forms of attack - one of the first, and easiest, is email injection attacks. Input can be crafted to do all sorts of things - create a BCC header when you don't have one, so you get one email, the target gets thousands, create a multipart body so the original email is basically junked but the multiparts can do anything they want - and probably a thousand other things I don't even know about.

The tools in PHP will capture *most* of the moderate threat attacks, but it still doesn't beat the basic defense: accept only what you expect and throw everything else away. A basic example is when you expect an integer, take only the integer part of the input. If it needs to be a specific integer, validate it against values server side. This requires a bit of tweaking, examining every point of input and controlling it ruthlessly, but it's well worth it.

The integer example is a good one, it will make your job easier. I often see people doing this,

select * from listings where neighborhood='Upper West Side'

Which come from say, a drop-down list of textual values. Being text, there's so many nasty things you can do with it that require more cleansing. But if you have . . . .

<select id="neighborhood" name="neighborhood">
<option value="1">Downtown</option>
<option value="2">Manhattan</option>
<option value="3">Upper West Side</option>
</select>

Then this one liner protects this field.

$neigborhood = preg_replace('/[^\d]/','',$_POST['neighborhood']);

(replace anything not a digit with empty string)

Or if you prefer,
$neigborhood = int('$_POST['neighborhood']);

This is better on two points, it's easier to cleanse and your database will now query much faster (integers!)

The reason for my pontification is that securing input is not a one-off, there's no single command that will protect you, and each vulnerability needs to be addressed individually. Sometimes the solution involves re-thinking your approach in ways that improve other aspects of your programming, using the example above.

Every input is a potential hack. Every input is a potential hack. Every input is a potential hack. - Selena Sol

[jamie]

<select id="neighborhood" name="neighborhood">
<option value="1">Downtown</option>
<option value="2">Manhattan</option>
<option value="3">Upper West Side</option>
</select>

that's a really good tip there. i already do this, but it's worth highlighting.

the op was really about the fact that i do filter input, but was not even aware that GET/SERVER params could be used to inject into pages. the PHP_SELF exploit was completely new to me.

afaik rasmus didn't even know about it when it came out :)

thanks all for the tips above

[Readie]

To respond mostly to Rocknbil - what I like to do with things like select menus is have an array of all my *expected* values, setup something like this:

$expected_vals = array(
  0 => true,
  13 => true,
  'string_key' => true
);

And then I can do

if(isset($expected_vals[$_POST['select_menu']])) {
  // some processing
} else {
  $errors[] = htmlentities($_POST['select_menu'], ENT_QUOTES) . ' is not a valid input.';
}

[enigma1]

And then I can do
......
$errors[] = htmlentities($_POST['select_menu'], ENT_QUOTES) . ' is not a valid input.';
.......

Actually no, you trust the user input by doing this, and you pass it directly through the htmlentities function. So if the 'select_menu' is not the type the function expects, it may generate a warning ie: path disclosure vulnerability.

eg:
$_POST['select_menu'] = array();
$errors[] = htmlentities($_POST['select_menu'], ENT_QUOTES) . ' is not a valid input.';

// Output
Warning: htmlentities() expects parameter 1 to be string, array given in 'physical path to site'

" is not a valid input."

[Readie]

Ahh, yes, I can see your point there, although I always have error reporting off so it isn't an issue for me. I suppose the way forward there is to maybe do this:

$errors[] = htmlentities(strval($_POST['select_menu']), ENT_QUOTES) . ' is not a valid input.';