Need some PHP security tips

[twist]

The first thing I did was put all my scripts but one in a .htaccess 'deny from all' directory. The script outside the directory is simply a one line script with an include statement.

I use RewriteRules to call the script. The RewriteRules only allow the selected text or number to be used. As far as I can tell, for example, if any word but those listed in the RewriteRules is used then it directs the person to an error page. So a person cant type mysite.com/someword and get anything but an error message.

Besides the short list of allowed words the only other variables I use are numbers (ie [0-9+]). When the script starts the first thing I do is check the number being passed. For example, if number!>0 or number!<6, it will just give the person the default page.

If the number being passed is a reference to a row in a database, the first thing I do is ask the database if that row exists. If it does, I continue with the script, if not, I give them the default page.

My questions are,

1) Will using strict RewriteRules and checking all incoming numbers stop buffer overflow attacks? For example, if a person tried passing the number googleplex and I did a database search to see if that row in the database existed could this wreak havoc, or would the length of the number be limited to the GET statements max length?

2) I notice some people get variables by using ($_GET). Why use this? Is it necessary for security?

3) I use

define('SOME_VARIABLE', true);

and then

if (!defined('SOME_VARIABLE')) { die('Page unavailable'); }

. Is this necessary since my pages are behind a 'deny from all' directory?

4) Any other major areas I am missing?

[mincklerstraat]

1) - I'm no expert on buffer overflow attacks so I'll leave this for someone else
2) Using

$_GET[]

and

$_POST

aren't necessary for security, but they sure are handy. It means that you can set register_globals to Off (which is now the default in PHP, though most hosts have it on - I just d/l'd XAMPP yesterday (looks very nice btw), and noticed this also has register_globals on - you can turn it back off in an .htaccess file if you can't get at php.ini). When register_globals are on, any variable you haven't properly initialized (set the value of it before using it) will be fair game to hackers - they can set any of these variables, or even elements of arrays, with simple http requests. If you develop with

error_reporting(E_ALL);

you're less likely to be a victim of vulnerabilities due to uninitialized variables; however, there's no guarantees even here - very likely you at some point do something like:

if(!empty($var_x)) do_something($var_x); 

And this basically amounts to the same thing as having an uninitialized variable, only it slips through E_ALL error detection. Another good reason to avoid using empty() unless you really need it.
3. You probably could avoid doing this if you're confident this directory is really blocked from all direct requests. You don't even need to do this if it's a file that can't really do much by itself (like, e.g., a file that just sets a lot of language variables). The place it's really dangerous is when you include something based on a variable. Again, having register globals off helps, since people can't just call this file with the variables set via GET in the request. Btw, stuff you define with

define()

is called a CONSTANT, not a variable (it can't be changed once it's set, so it isn't variable. Since it can't be changed, it's available everywhere, inside functions too).
4. First things that jumps to mind are:

file permissions

checking to see if magic_quotes_gpc() is on or not, and if so, stripping slashes from all request variables;

putting quotes around all the stuff that goes into query strings that comes from variables.

$query = 'SELECT * from mytable where text="'.$text.'"'; //note the double quotes

using

htmlspecialchars()

or otherwise filtering the output of anything that comes in from the user and is destined for display in HTML.

Cross-site request forgeries are relevant to some sites, but are a bit more complex - Chris Schiflett is one of the best writers on php security, you can read his article at [shiflett.org...] - his other stuff on security will be worth your while, too, better than what 'ole minck can just think of off the top of his head.

[ergophobe]

Above all, I just want to second minck's suggestion of reading Chris Shifflet's stuff. If you find it useful, drop him a line too so he'll keep making it available. I think he gets a lot of readers, but very little feedback.

[twist]

Using $_GET[] and $_POST aren't necessary for security, but they sure are handy. It means that you can set register_globals to Off. You can turn it back off in an .htaccess file if you can't get at php.ini.

I added this line to my .htaccess,

php_flag register_globals off

but according to phpinfo() register_globals is still on...?

stuff you define with define() is called a CONSTANT

I knew that at some point, thanks for the reminder.

checking to see if magic_quotes_gpc() is on or not, and if so, stripping slashes from all request variables;

according to phpinfo(),
magic_quote_gpc() = off
magic_quotes_runtime() = off
magic_quotes_sybase() = off

Should I turn any of these on or just be concerned with magic_quote_gpc?

# Cross-site request forgeries are relevant to some sites, but are a bit more complex - Chris Schiflett is one of the best writers on php security, you can read his article at [shiflett.org...] - his other stuff on security will be worth your while, too, better than what 'ole minck can just think of off the top of his head.

Great article

[twist]

Not to get ahead of myself, but here is what I have tried so far,

I added the code from this page [us4.php.net...] that was submitted by alex at sourcelibre dot com just a ways down the page. It says "This is a simple function to make the register globals OFF and magic_quotes_gpc ON." If anybody knows any drawbacks to this code, please let me know.

I added error_reporting(E_ALL);

and this is my basic setup so far,

if(!empty($_GET['variable'])) { $name = $_GET['variable']; }
else( $name = 'default_1'; }
if( $name == 'default_3' ) { ... }
else if( $name == 'default_2' ) { ... }
else { $name = 'default_1' }

Thanks for all the suggestions so far, they have been great.

[twist]

* I was just going to delete my last post and write a new one, but my edit button has disappeared?

Anyway, I am curious about overkill, example,

RewriteRule ^([a-z])$ /page.php?letter=$l [L]

This means if a person tries to use any variable but a-z it will not open the page, correct?

So, if I get the variable like so,

if(!empty( $_GET[ 'letter' ] ) ) {
if( strlen( $_GET[ 'letter' ] ) > 1 ) { $letter = 'a'; }
else { $letter = $_GET[ 'letter' ]; }
}
else { $letter = 'a'; }

Will the above be enough or should I also check every letter again in php?

if( $letter!= 'a' && $letter!= 'b' and so on, or is this overkill? In fact, is my above if statements overkill also? Do I really need to check the string length even though anything over 1 char will not open the page anyway?

[mincklerstraat]

Hey Twist

A good idea to put

php_flag register_globals off 

in your root .htaccess file. That'll turn it off there. When you look at phpinfo, you should see two columns for this value - a 'local value' and a 'master value' - this will only change that 'local value.'

One disillusioning note: there are some scripts out there that still require this value to be On. If you ever have to run one of those scripts, you can just put the script in a directory of its own, and put an .htaccess file there with php_flag register_globals on

About magic_quotes_gpc() :
This was something of a "security feature" in PHP that many now regret ever existed, since it creates an extra layer of complexity and need for checking. What it does is adds slashes in front of quotes, so if you happen to send a query to your database without mysql_escape_string() or mysql_real_escape_string (good I think of that, it belonged in my first post, will get back to it later), and it had quotes in it from user input, all that input would have slashes in front of the quotes, so it wouldn't break things or allow hackers to do nasty stuff.

Anyways, it can hamper security too, if you aren't taking account of it, and can lead to exploits as well.

The best practice is, whether you have it on or not, to check to see if it's on, and if it is, to stripslashes() from all user input that might possibly have quotes in it (or if you're not sure, just everything). You can check if it's on easily with magic_quotes_gpc(). The other magic quotes flags have other functionality for other types of quote escaping; leave them off. Whether you have magic_quotes_gpc on or not shouldn't matter - you should write your scripts so they run the same way if it's on, or if it's off.

About mysql_escape_string: yes, do this for every string that goes into a query that contains user input. It escapes quotes, and will keep hackers from adding quotes to a query to then add their own commands to your query strings.

And ok, your last question, controlling input: fortunately, you don't have to the big horrendous if thing. Imagine if you had to take $_GET parameters that were up to sixteen characters long? you'd have 12*16 if statements.

You are on the right track, though, checking your user input. You should definitely be doing this.

You can check your input with things called regexes. These are difficult at first, but when you learn to use them, you'll have so much power available at your fingertips. Use the preg_ family instead of the ereg family - preg is faster and more powerful.

if(!empty($_GET['letter'])){ 
if(preg_match('#^[A-Za-z0-9]$#', $_GET['letter']) { 
$letter = $_GET['letter']; 
} 
else { die('That\'s not alphanumeric input!'); 
} 

This checks to see if the character is composed only of uppercase letters (A-Z), lowercase letters (a-z), and numbers (0-9). E.g., if you don't want it to contain numbers, delete the 0-9 part (delete, don't replace with spaces, this bit is space sensitive).

If regexes are confusing you you could also use the ctype functions (check character type - [us4.php.net...] which are a lot more straight-forward, but learning regexes is really a must for serious php programming and you'll be so happy when you know them well.

Note: that technique of putting the 'Rewrite Rule' in your .htaccess file also uses a regex. However, I'd use this way, and not use rewrite rules except for where you really need them. This way keeps all your stuff in one file where you understand what's happening if you ever have to move around. Having stuff divided between code and htaccess files means an extra degree of complexity and difficulty in keeping your code up-to-date.

Happy coding!

[twist]

Hey, wow thanks for all the help. I am familiar with regular expressions from the .htaccess file but was unaware you could just plug them into php. This will save me a lot of time. Really glad I started this thread, have learned a lot.