Re: Sessions...

Greg Donald wrote:

I've been using this database driven PHP sessions setup for years: http://dbsessions.destiney.com/

I offer a similar implementation: http://phpsecurity.org/code/ch08-2 A few notes about yours (if you don't mind the critique): 1. It doesn't use a separate database connection, but it creates one anyway. I create a separate one, but only so that it can be dropped into any web application without adversely affecting it. In practice, I prefer to use a single connection. 2. Your sess_close() function does nothing, and this can make the session mechanism behave unexpectedly for a developer. For example, someone who actually depends upon session_write_close() for concurrent connections is severely impacted. 3. $GLOBALS[tb_sessions] is invalid syntax. It works, but only because PHP will check to see if you meant $GLOBALS['tb_sessions'] after it fails to find a constant named tb_sessions. 4. Your sess_read() function should return an empty string when no data is found, not FALSE. I originally made this same mistake. 5. Your sess_read() function has an SQL injection vulnerability. The argument passed comes directly from the user. 6. Your sess_write() function has the same SQL injection vulnerability. 7. In sess_write(), you also don't escape the session data, and this will give you problems, even with legitimate data that doesn't represent an SQL injection attack. For example, try setting a session variable that contains a quote somewhere:

    $_SESSION['publisher'] = "O'Reilly";

8. Your sess_write() function has the same SQL injection vulnerability. 9. Your sess_destroy() function has the same SQL injection vulnerability. 10. Your sess_gc() function returns the number of affected rows. Shouldn't it return TRUE on success? You may just be depending upon the evaluation of any non-zero integer to TRUE. 11. Your sess_gc() function ignores the session configuration and chooses its own expiry. I hope this was helpful. I don't mean any disrespect - quite the opposite, since you're freely sharing your code. I applaud that. :-) I think the SQL injection vulnerability is the biggest flaw. Most everything else doesn't matter too much, but that's a big one, and it exists in most of your session functions. A simple call to mysql_real_escape_string() can protect against this vulnerability (in these cases), and you'll see that I use this function on everything I use in my SQL queries, even when it seems ridiculous to do so:

    $access = time();
    $access = mysql_real_escape_string($access);

Hope that helps. Chris -- Chris Shiflett Brain Bulb, The PHP Consultancy http://brainbulb.com/