Mejorar Sistema de Seguridad

Hola, estoy haciendo un codigo de seguridad para un sistema que estoy armando así que me gustaría que me ayuden a mejorarlo por eso lo subí a mi hosting y así lo pueden probar les pido por favor que me digan si encuentran un fallo o alguna manera de mejorar el código, ya que seria de gran ayuda para mi y me serviría para mejorar mis conocimiento en php que recién estoy empezando , desde ya gracias.

http://maximilianogarro.com.ar/

El código en cuestión:
fns_login.php

Código PHP :

//Reporte de error
    function error($error){
        setcookie("reporte",$error,time()+180,"/","");
        header("Location: ../index.php");
        exit;
    }
//Conectamos a la base de datos
    function db_login(){
            //Conectamos a la db
            $host = '------';
            $userdb = '------';
            $passdb = '------';
            $db = '------';               
            $connect = @mysql_connect($host,$userdb,$passdb);
            if($connect === false){
                return false;
                    error("Error: No se a podido conectar a la base de datos.");
            }else{
            //Si se conecto seleccionamos la db
                $selectdb = @mysql_select_db($db,$connect);
                if($selectdb === false){
                    return false;
                    die('Error: No se a podido conectar con la base de datos.');
                }else{ return true; }
            }
            
    }
 
//Encriptar con contraseña
    function Encriptar($pass){
        $passsafe = hash_hmac('sha1', $pass, '_?¿·¨^*ç"$"1@ª!"·¿?·¡523ç´MaXi´Ç¨ç{}');
        return $passsafe;
    }
    
//Verifica si esta logeado.
    function firewall(){
        //verificamos que existan las cookies
        if ( isset($_COOKIE['sistema']) && isset($_COOKIE['sistema2']))
        {
            //Inicializamos VariablesS
            $key = $_COOKIE['sistema'];
            $userid = $_COOKIE['sistema2'];
            //Conectamos ala db
            $connect = db_login();
            //Si la selecciono procedemos con la consulta
            if ($connect === true){
            $loginquery = sprintf("SELECT * FROM `usuarios_sess`,`usuarios_log` WHERE `usuarios_sess`.`fkusuario`= '%s' AND `usuarios_log`.`pkusuario_log` = `usuarios_sess`.`fkusuario_log`;", mysql_real_escape_string($userid));                          
            $login = @mysql_query($loginquery);
                
                if (($login === false) || (mysql_num_rows($login) != 1)){
                        error("Error: Intente nuevamente.");
                }else{
                    $usuariodb = mysql_fetch_array($login);
                    $keydb = $usuariodb['KEY'];
                    $ip = $usuariodb['ip'];
                    //Comparamos Key e IP para evitar que nos roben la cookie :D
                    if ($keydb != $key || $ip != $_SERVER['REMOTE_ADDR']) {
                        error("Error: Debe logear para ver esta pagina.");
                    }
                }
            mysql_close();
            }
        }else{
            header("Location: index.php");
        }
    }

login.php

Código PHP :

    //Iniciamos Session
    session_start();    
    
    //Incluimos funciones
    include_once('fns_login.php');
    //Validamos que las variables del captcha existan
    if (!isset($_POST['key']) || !isset($_SESSION['CAPTCHA'])){
        error("Error: Intenta acceder a este sitio de una forma incorrecta.");
    }else{
        //Validamos que las varibles encriptadas sean iguales
        if(md5($_POST['key']) != $_SESSION['CAPTCHA']){
            error("Error: Nos has introducido el codigo correcto.");
        }else{
            //Destruismos variables captcha
            unset($_SESSION['CAPTCHA']);
            unset($_POST['key']);
            session_destroy;
            
            //validamos variables
            $user = $_POST['user'];
            $pass = $_POST['pass'];
            
            //Encriptamos password con sha1 y contraseña
            $passsafe = Encriptar($pass);
        
            if (!isset($user) && (ereg("^[a-zA-Z0-9]$", $user)=== false) && (strlen($user)> 50)){
                error("Error: Nombre de usuario invalido.");
            }else{
                if (!isset($pass) && (ereg("^[a-zA-Z0-9]$", $pass)=== false) && (strlen($pass)> 20)){   
                    error("Error: Contraseña con caracteres invalidos.");
                }else{      
                            //Conectamos ala db
                    $connect = db_login();
                    //Si la selecciono procedemos con la consulta
                    if ($connect === false){
                        error("Error: No se pudo conectar con la base de datos.");
                    }else{
                        //Si la selecciono procedemos con la consulta
                        $loginquery = sprintf("SELECT `pkusuario` FROM `usuarios` WHERE `usuario`='%s' AND `password`='%s'",
                                                mysql_real_escape_string($user),
                                                mysql_real_escape_string($passsafe)
                                                );                           
                        $login = @mysql_query($loginquery);
                            
                        if (($login === false) || (mysql_num_rows($login) != 1)){
                            error("Error: El usuario no existe.");
                        }else{
                            // Ya logio => obtenemos datos
                            $usuariodb = mysql_fetch_array($login);
                            $userid = $usuariodb['pkusuario'];
                            $fecha = gmdate('Y-n-j H:i:s');
                            $ip = $_SERVER['REMOTE_ADDR'];
                            $localhost = gethostbyaddr($ip);
                            $navegador = $_SERVER['HTTP_USER_AGENT'];
                            
                            //encriptamos un key unico de usuario
                            $key = Encriptar($ip.$fecha);
                            
                            //almacenamos logs por seguridad y consulta futura
                            $logs = "INSERT INTO `usuarios_log` (`pkusuario_log` ,`fkusuario` ,`ip` ,`localhost` ,`navegador` ,`login` ,`logout`)
                                            VALUES ('null', '".$userid."', '".$ip."', '".$localhost."', '".$navegador."', '".$fecha."', '".$fecha."');";                                
                            $almacenalogs = @mysql_query($logs);
                                
                            $idlogs = mysql_insert_id();
                                
                            //borramos si existia session
                            @mysql_query("DELETE FROM `usuarios_sess` WHERE `fkusuario` = '".$userid."'");
                                
                            //entonces alamacenamos los nuevos keys 
                            $keylogs = "INSERT INTO `usuarios_sess`(`KEY` ,`fkusuario` ,`fkusuario_log`)
                                               VALUES ('".$key."', '".$userid."', '".$idlogs."');";
                                                
                            $almacenakeylogs = @mysql_query($keylogs);
                                
                            if (($almacenalogs === false) && ($almacenakeylogs === false)){
                                error("Error: No se puede iniciar existe un error al conectar.");
                            }else{                                  
                                setcookie("sistema", $key, time()+3600,"/","");
                                setcookie("sistema2", $userid, time()+3600,"/","");
                                header("Location: ../web_segura.php");
                                mysql_close();
                                die;
                            }
                        }
                    }
                }
            }
        }
    }

logout.php

Código PHP :

include_once('fns_login.php');
 
if ( isset($_COOKIE['sistema']) && isset($_COOKIE['sistema2'])){
    
        $key = $_COOKIE['sistema'];
        $userid = $_COOKIE['sistema2'];
        
        setcookie("sistema", $key, time()-3600,"/","");
        setcookie("sistema2", $userid, time()-3600,"/","");
        
        $fecha = gmdate('Y-n-j H:i:s');
        $connect = db_login();
        
        //Si se conecto a la db
        if ($connect === true){                 
            $logoutquery = sprintf("UPDATE `usuarios_log` SET `logout` = '".$fecha."' WHERE `pkusuario_log` = (SELECT `fkusuario_log`
                                    FROM `usuarios_sess` WHERE `fkusuario` = '%s' );" , mysql_real_escape_string($userid));                          
            $logout = @mysql_query($logoutquery);
            
            $keysess = sprintf("DELETE FROM `usuarios_sess` WHERE `fkusuario` = '%s';", mysql_real_escape_string($userid));
            $keyout = @mysql_query($keysess);       
            mysql_close();
        }
        if (!isset($_COOKIE['sistema']) || $keyout === true){
            setcookie("reporte","Deslogeado Correctamente.",time()+180,"/","");
        }
        header("Location: ../index.php");
    }else{
        header("Location: ../index.php");
    }

Link de descarga del codigo:
http://www.4shared.com/file/_eUj3D8u/userlogin_by_beringer.html

Me detengo en esta parte.

Código PHP :

//validamos variables 
   $user = $_POST['user']; 
   $pass = $_POST['pass']; 

//Encriptamos password con sha1 y contraseña 
   $passsafe = Encriptar($pass); 

   if (!isset($user) && (ereg("^[a-zA-Z0-9]$", $user)=== false) && (strlen($user)> 50)){ 
      error("Error: Nombre de usuario invalido."); 
   }else{
   ....

Verificar que la variable $user existe es inútil porque tal como está el código, esa variable siempre existirá independientemente de lo que ocurra con $_POST['user'] pues el solo hecho de asignarle algo (lo que sea) ya provoca que exista. Lo mismo pasa con $pass.

Creo que lo que has querido hacer es verificar que no esté vacía. Esta es mi propuesta:

Código PHP :

//validamos variables 
   $user = trim(@$_POST['user']); 
   $pass = trim(@$_POST['pass']); 

//Encriptamos password con sha1 y contraseña 
   $passsafe = Encriptar($pass); 

   if (!empty($user) && (ereg("^[a-zA-Z0-9]$", $user)=== false) && (strlen($user)> 50)){ 
      error("Error: Nombre de usuario invalido."); 
   }else{
   ....

Un comentario. Por lo que se ve en la expresión regular, no admites vocales acentuadas.

Ahora en general.
Veo que usas cookies para transportar el mensaje de error de una página a otra. Es mejor si usas sesiones. Como las cookies reciden en la máquina del visitante, se presta para que abusen de ellas y puedan enviarte datos que no esperas recibir.

Hasta aquí lo que puedo ver. Estoy seguro que otro colega verá otras cosas para mejorar este código

bueno en esta parte ejej, es solo una teoria

Código PHP :

                            $navegador = $_SERVER['HTTP_USER_AGENT']; 
                             
                            //encriptamos un key unico de usuario 
                            $key = Encriptar($ip.$fecha); 
                             
                            //almacenamos logs por seguridad y consulta futura 
                            $logs = "INSERT INTO `usuarios_log` (`pkusuario_log` ,`fkusuario` ,`ip` ,`localhost` ,`navegador` ,`login` ,`logout`) 
                                            VALUES ('null', '".$userid."', '".$ip."', '".$localhost."', '".$navegador."', '".$fecha."', '".$fecha."');";      

especificamente en esta linea

Código PHP :

                            $navegador = $_SERVER['HTTP_USER_AGENT']; 

esa variable viene dada por el navegador del cliente la cual puede ser manipulada, y dicah variable tiene acceso a la base de datos , talvez algun experto en SQL Injection le sacaria algun provecho
por ejemplo yo usaba un script para hacer un envio automatico por metodo post a una web X
y el enviara estas cabeceras

Código PHP :

$enviar_respuesta = array(
"POST /ConsultaEDM2010Nal/wfrmConCiudadanoP.aspx HTTP/1.0",
"Host: www.cne.org.bo",
"User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; es-MX; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10",
"Accept-Language: es-bo,en-us;q=0.7,en;q=0.3",
"Keep-Alive: 115",
"Connection: keep-alive",
"Referer: http://www.cne.org.bo/ConsultaEDM2010Nal/wfrmConCiudadanoP.aspx",
"Content-Type: application/x-www-form-urlencoded",
"Content-Length: %s");

el punto es que ahi yo especifico el User-Agent el cual podria manipular a mi gusto,aunque es solo teoria lo de realizar un iSQL

Hola, muchas gracias por su respuesta son muy interesantes y me ayudan a aprender un montón ya que soy novato , no había pensado lo del envió de cabeceras , voy a investigar bastante sobre eso, alguna otra opinión?


Saludos.

algunas funciones estan obsoletas en version php5.3 y superiores las cuales te darian dolores de cabezas,
la funcion ereg que estas usando
echa un vistazo

http://es2.php.net/ereg

¿Que función podria usar y como se utilizaría? preg?

(preg_match("/^([a-zA-Z0-9])+/", $user) ?

Sí.

Código PHP :

if (preg_match("/^([a-zA-Z0-9])+/", $user) === 0) {
    echo "Nombre de usuario incorrecto";
}

La función devuelve un int.

Ahora bien, debes considerar una cuestión de usabilidad porque tal como está la expresión regular no admites vocales acentuadas.

También una cuestión de seguridad. En el código original verificas que el nombre de usuario o la contraseña no sea mayor a 50 caracteres, lo cual está bien, pero no controlas el mínimo. La expresión regular admitiría un nombre de usuario tal como "0" (cero) y contraseña "0".

Muchísimas gracias por su ayuda