Skip

PHP Security
March 30, 2006 11:36 PM   Subscribe

I’m working on a PHP/MySQL app and would like to ensure my security is up to scratch – need tips on authentication, globals and input sanitization.

My current method of authenticating users is a simple MySQL username/password lookup, then storing their state with a session and a required cookie (which stores only the session id). To prevent fixation I am using session_regenerate_id whenever necessary. Am I missing anything?

Register globals is on by default. I am not using globals, and I am trying to define all variables before use. Am I safe? Can global hacks affect my sessions?

My current method of input sanitization is:

1. strip < ,>, ‘\r’ and ‘\n’ to prevent scripting attacks
2. convert everything to entities
3. escape anything left with mySQL_real_escape

Is this sufficient to protect against any/all injection/xss attacks?

PHP Security is giving me a big headache, and I keep feeling like I’m missing something important. Any tips, corrections, best practices or links would be very much appreciated.
posted by MetaMonkey to Computers & Internet (28 answers total) 3 users marked this as a favorite
 
I am not using globals, and I am trying to define all variables before use.

Just turn off register_globals. You don't need it for anything else but legacy apps.

convert everything to entities

This is crap. You need to work out which strings are text (without entities) and which are HTML (with entities). The best thing to do is to store everything as text and convert it to HTML on output.
posted by cillit bang at 12:05 AM on March 31, 2006


Just turn off register_globals. You don't need it for anything else but legacy apps.

I should have said, I don't think my host will let me turn off globals. I should double check though.

You need to work out which strings are text (without entities) and which are HTML (with entities).

Why? I'm not planning on allowing any html, at least for the time being. Converting to entities seemed like a safe way to dodge mysql injection and cross-site scripting. I was also considering using custom html translators (e.g. [bold] being parsed to >b<) when I get around to it.

The best thing to do is to store everything as text and convert it to HTML on output.

I'm not sure what you mean by this. Where does the security come in? Is mysql_real_escape sufficient?
posted by MetaMonkey at 12:17 AM on March 31, 2006


What does your code for doing the password lookup look like?
posted by Leon at 12:50 AM on March 31, 2006


Why? I'm not planning on allowing any html, at least for the time being. Converting to entities seemed like a safe way to dodge mysql injection and cross-site scripting.

Text entered in a form by a user is plain text. When you display that text back to the user certain characters must be escaped to avoid it being interpreted as HTML. That's obvious. What most developers don't think through properly is when that conversion happens.

Escaping HTML/stripping tags before you put it in the database is a crap strategy that too many people use. Conceptually, instead of storing the user's actual data in the database, you're throwing it away and only storing pre-rendered output data. That's fine if the only thing you ever plan to do with your data is echo it as part of HTML page, but if you decide you need to do anything else with it (print it, use it in plain text emails, copy to another database etc), you're screwed.

Also, if you're not allowing HTML in the input, you should not strip angle brackets. If the input is plain text then angle brackets entered there should also appear in the output. I should be able to type "I <really> love your site" and not have any part of it disappear. This isn't directly a security issue, but if you design your site so that this works instead of panicking every time you see a user enter an angle bracket, you'll do a better job of dealing with this issue.
posted by cillit bang at 12:51 AM on March 31, 2006


Use mysqli so single queries can't be made into multiple queries.
posted by nonmyopicdave at 1:07 AM on March 31, 2006


Leon: What does your code for doing the password lookup look like?

About as straightforward as you can get:

User inputs name & password.
Run a MySQL query to see if there is match on username.
If the username matches, check for match on password for that row. If that matches, they're in.

cillit bang:

What you say makes sense, I hadn't considered the utility of having the text unaltered. So, I can do conversion to entities at the point of displaying the data, taking care of cross-site scripting attacks.

But what of MySQL injection attacks - will mysql_real_escape_string make my data safe to input?
posted by MetaMonkey at 1:20 AM on March 31, 2006


will mysql_real_escape_string make my data safe to input?

Yes. Make sure you enclose every piece of data in quotes too - eg (in bold) "WHERE foo='".mysql_real_escape_string($bar)."'"
posted by cillit bang at 1:28 AM on March 31, 2006


But what of MySQL injection attacks - will mysql_real_escape_string make my data safe to input?

Possibly not, which is why I wanted to see the code you're using.

(BTW, just use a single query looking up rows where username=$username and password=$password).
posted by Leon at 1:30 AM on March 31, 2006


Thanks very much cillit bang, that is all very useful.

Allow me to confirm that what you have written is equivalent to my method,

$bar = mysql_real_escape_string($bar);
$query = " SELECT * FROM table WHERE foo='{$bar}' ";


nonmyopicdave: Use mysqli so single queries can't be made into multiple queries.

Sounds good, I will look into it, thanks.
posted by MetaMonkey at 1:38 AM on March 31, 2006


Leon:

What code is it you are interested in? I thought you were talking about the log-in, not the sanitzation bit.

All in all, with your tip taken into account, it would look something like this:

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string($_POST['password']);

$query = "SELECT username, password FROM table WHERE username='{$username}' AND password='{$password}'";

if (@mysql_query ($query)) {
//login session stuff
} else {
print'username of password does not match';
}
posted by MetaMonkey at 1:46 AM on March 31, 2006


Actually, come to think of it I use

$var = ereg_replace("[^a-z,A-Z,0-9, ,]","",$var)

on the username and password to be extra careful.
posted by MetaMonkey at 1:49 AM on March 31, 2006


You're going to get E_NOTICE warnings if the username or password don't exist in the POST. You probably don't care about that. If you do, try:

function getPost($variable) {
return (isset ($_POST[$variable]) ? $_POST[$variable] : NULL);
}

$username = getPost('username');

Other than that, looks fine as far as I can see. The {} are unnecessary, but some people seem to prefer them for readability/consistency reasons.
posted by Leon at 2:19 AM on March 31, 2006


I second the mysqli recommendation as a replacement for mysql_real_escape_string. the absolute best way to avoid sql injection is using a parameterized query. It's in PHP5 by default, but you have to have it installed as a plugin or somesuch for PHP4. And you can use it OOP style if you're into that.

Also, use stored procedures when you can.

(reference 1 2)
posted by escher at 2:41 AM on March 31, 2006


Mysqli sounds good, and it seems to be installed as an option in my version of php 4.

However, I seem to be able to use msqli_query, but not mysqli_connect. Can anyone help with using mysqli in PHP 4? Its a pretty tricky subject to google for.

Leon: thanks for the help and tips.
posted by MetaMonkey at 4:08 AM on March 31, 2006


There's tons of mysqli documentation at php.net.
posted by matthewr at 4:17 AM on March 31, 2006


There's tons of mysqli documentation at php.net.

As far as I can tell it all seems to be refering to PHP 5. I'm trying to figure out why I can do mysqli_query() but when I try mysqli_connect() I get a 'function not defined' error.
posted by MetaMonkey at 4:20 AM on March 31, 2006


You say you can do mysqli_query - what are you putting in as the first parameter? What does mysqli_query return? What does extension_loaded("mysqli") return?
posted by matthewr at 4:26 AM on March 31, 2006


No, my mistake. mysqli_query() does not work, and extension_loaded("mysqli") returns false.

So I presume I'll either have to change host or do without mysqli.
posted by MetaMonkey at 4:47 AM on March 31, 2006


Nearly everyone survives without mysqli so it's not a great loss. mysql_real_escape_string does the job. I don't think you can use stored procedures without mysqli though.

If you're using $_SERVER["REQUEST_URI"] anywhere, remember to run htmlentities() on it to protect against cross-site scripting attacks.
posted by matthewr at 4:53 AM on March 31, 2006


Thanks for the help matthewr. It's not really that big of a deal, I just started to get interested in stored procs and all that jazz. I'm quite new to this PHP/SQL malarky. I guess I'll worry about it if anyone actually uses the damn thing.

I think my question has largely been answered now, the only thing I'm still a bit worried about is authentication using sessions.

Currently, after validating the user at login I set

$_SESSION['logged_in']='true'

Then, every time a page is requested or action is performed that requires authentication I do

session_start();
session_regenerate_id();
if ($_SESSION['logged_in']=='true') {
//do authorised stuff
} else {
exit('you must be logged in to do that');
}


Is this sufficient to prevent the evil ones from sneaking into my system? Users are going to be able to INSERT and UPDATE, and possibly, eventually DELETE, so I'd rather not have people spoofing sessions and suchlike.
posted by MetaMonkey at 5:07 AM on March 31, 2006


I second the mysqli recommendation as a replacement for mysql_real_escape_string. the absolute best way to avoid sql injection is using a parameterized query.
This is extremely good advice. I have to cringe whenever people do all this manual and potentially error-prone escaping when they could just use parameterized queries. They also look much nicer... in languages where proper MySQL modules was not an afterthought like PHP. Compare in perl:
$dbh->do("UPDATE userDB SET foo=?, bar=? WHERE user=?", undef, $foo, $bar, $user);
vs.
$dbh->do("UPDATE userDB SET foo='" . $dbh->quote($foo) . "', bar='" . $dbh->quote($bar) . "' WHERE user='" . $dbh->quote($user) . "'");
Nearly everyone survives without mysqli so it's not a great loss. mysql_real_escape_string does the job.
Yes of course. This is a perfect example of what turns people off from PHP. It's just not clean. It's a dirty little combination of crap that was thrown together without thought or real design. The familiar "mysql" module doesn't even have the capability to do parameterized SQL -- somebody must have said, "nah, doesn't sound important, it's not something I use, so why bother." And of course that is the module that became de-facto, so everyone uses it to the point where the rewritten and "improved" mysqli is almost impossible to find on stock systems. Mediocrity prevails, and consequently people grow up on PHP thinking that escaping is something that has to be done manually (and is therefore often neglected.)
posted by Rhomboid at 5:48 AM on March 31, 2006


it looks like you are storing plaintext passwords in your DB thats a nono, you need to store a hash of the password and then compare a hash of the submitted password to the DB value
posted by BSummers at 6:22 AM on March 31, 2006


I don't think my host will let me turn off globals

I'd say get a new host if that's the case and you have security concerns. Auto-globals aren't inherently a security risk, but they make it much too easy to create a security risk.

the only thing I'm still a bit worried about is authentication using sessions

Sessions IDs are relatively trivial to intercept over HTTP. Most larger sites require sessions to be re-authenticated (i.e. type your password again) before any security-sensitive action (e.g. password change) can be taken.
posted by scottreynen at 7:07 AM on March 31, 2006


You should be able to turn off register globals with a directive in your php.ini, without affecting anyone else on your shared host.
posted by evariste at 12:07 PM on March 31, 2006


BSummers: you need to store a hash of the password and then compare a hash of the submitted password to the DB value

Thanks, I had considered that but hadn't got around to it. I will do that. I also considered two-way and possibly one-way encryption, is this necessary or overkill for my needs?

scottreynen: Sessions IDs are relatively trivial to intercept over HTTP. Most larger sites require sessions to be re-authenticated... before any security-sensitive action (e.g. password change) can be taken.

Re-authentication for high-risk actions sounds reasonable. I am still concerned how to minimise general session capture risks. But I suppose everyone else seems to live with it, so it probably isn't that critical right now.

evariste: You should be able to turn off register globals with a directive in your php.ini, without affecting anyone else on your shared host.

Thanks evariste, I've will look into that.

Rhomboid, parameterized queries sound like exactly what I should be using. But PHP 4 is what I got right now, and has made it farily easy for me to hack something together learning as I go along. I was thinking to switch to one of those cool languages like Python or Ruby at some point, does anyone know if they handle mysql/security any better, generally?
posted by MetaMonkey at 1:42 PM on March 31, 2006


Huge thanks to all the answerers here so far, this thread has taken care of many of my concerns.

I am quite astonished how hard it is to find any kind of comprehensive PHP security advice on the net (written for the general reader), or even in a book. I, and probably many others would be quite willing to pay money for something authorititive.
posted by MetaMonkey at 1:47 PM on March 31, 2006


I am quite astonished how hard it is to find any kind of comprehensive PHP security advice on the net (written for the general reader), or even in a book.
One way to learn about these things is to study vulnerabilities in existing PHP software that have been reported. Here is a good place to start. Also, consider selecting a package that historically has had a lot of vulnerabilities (phpBB is the quintessential example) and then search the archives of bugtraq or full-disclosure and look at the vulns reported against that package.
posted by Rhomboid at 12:43 AM on April 1, 2006


Thanks Rhomboid, that is most helpful.
posted by MetaMonkey at 8:06 PM on April 2, 2006


« Older I need help migrating to Linux...   |  The girlfriend has lost her se... Newer »
This thread is closed to new comments.


Post