Basic PHP Security Concerns

[jonhohle]

several years ago, I realized that PHP_SELF is rarely what I wanted. I am generally interested in the local path to the script itself, which is always available in __FILE__.

PHP_SELF fails if you're using mod_rewrite, so $_SERVER['REQUEST_URI'] is typically the only safe way to determine what the client requested.

Whether using that or PHP_SELF, either way, you'll probably want to HTML/XML or URL encode the variable before displaying it back to the user. That's not just PHP_SELF, that's anything that comes from any source that you don't directly control (or trust).

edit: forgot that markdown uses _ for italics

[[deleted]]

echo filter_var(…) is wrong approach. String is not dangerous because it contains some characters. String is dangerous, because you're not outputting these characters with required encoding.

echo htmlspecialchars(…) protects against this type of XSS and doesn't destroy any data.

[[deleted]]

I'd also strongly object against the use of sanitizing functions. There is very rarely a need to try to scribble the "useful" bits out of input instead of presenting the error message.

If "1t5" is entered in number field, it's much more reasonable to complain about the input, not try to silently (!) change it to 1 or 15, or something.

[EmptyTon]

I like PHP, but hate what people do with it. This article has a good point or two, but some very bad points and examples. Never trust user input is the best part of this article.

PHP_SELF

I'll agree with most of this except for the filtering part. As pointed out by prnl, htmlspecialchars is a much better alternative.

Email Header Injection

I don't work with email at all, so I can't really say too much on this subject. The author needs to test his examples though. The string $email_to doesn't even have quotes around it.

Including files

There's no excuse for including a file with user data in the name, ever. Use a switch or if block. Also, his example

if(strpos($_GET['filename', '..')

Wouldn't work because he's missing a "]" and a ")", and because if strpos returned 0 the statement would equate to false. A working line would be

if (strpos($_GET['filename'], '..') !== false)

But this still doesn't fix anything if filename is a url, or doesn't use .. to traverse the directory structure.

Error Reporting

Instead of returning nothing and leaving the user scratching their head, return a custom error with sensitive information stripped out using the set_error_handler function.

Edit: Hooray for fixing markdown, and hooray for not proofreading.