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/
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
