"Lorenzo Hernandez Garcia-Hierro" <[EMAIL PROTECTED]> tapota :
> Hi,
> At line 91 of /frontend/php/account/login.php :
>
> ## UNSECURE
> ##if ($session_hash)
> ##{
> ## #nuke their old session
> ## session_cookie('session_hash','');
> ## db_query("DELETE FROM session WHERE session_hash='$session_hash'");
> ##}
>
> possibly can be solved by doing something like this:
>
> =======================
>
> if ($_GET['session_hash'] ||
> $_POST['session_hash'] )
> {
> die("Confused.");
> }
> $session_hash = $_COOKIE['session_hash'];
> $session_hash = addslashes($session_hash);
> if (eregi('',$session_hash) ||
> eregi('[something for check if session is not a
> wildcard]',$session_hash))
> {
> die ("Confused.");
> }
> if ($session_hash)
> {
> #nuke their old session
> session_cookie('session_hash','');
> db_query("DELETE FROM session WHERE session_hash='$session_hash'");
> }
>
> =======================
According to <http://fr3.php.net/manual/fr/function.die.php>, the
fonction die "might be only in CVS" of PHP. We must not use function
that are not at least in PHP 4.0. The best is to use functions already
in PHP3.
> Tell me for hacks of this , to make it better , any idea for check SQL
> wildcards ( * , etc )
I think this was from the start a bad way to manage sessions. What we
need to implement is a clean interface, in my/ to manage session :
user should get a list of opened session (including IP) and they
should be able to trash old session only there.
Instead of adding plenty of regexp to take care of each malicious
string that can be passed (regexp are CPU time consuming, especially
with PHP) the best is to use clever SQL commands.
For instance, instead of blindly doing a "DELETE FROM session WHERE
session_hash='$session_hash'", it would be far more clever to do a
"DELETE FROM session WHERE session_hash='$session_hash AND
user='$user_id'", while $user_id is determined by the program and not
given via POST or GET.
That way, you can allow someone to pass whatever crap with the
session_hash, it will impact only his own session.
Apart from that, it would be a good time to implement a limit of
session number: to save database space, to avoid keep unused old
sessions (just an open door for user account cracking).
Regards,
--
Mathieu Roy
+---------------------------------------------------------------------+
| General Homepage: http://yeupou.coleumes.org/ |
| Computing Homepage: http://alberich.coleumes.org/ |
| Not a native english speaker: |
| http://stock.coleumes.org/doc.php?i=/misc-files/flawed-english |
+---------------------------------------------------------------------+