Ejercicio de la Semana: Login de usuarios y actualización de datos

A partir de ahora, cada semana haré el intento de poner los quiz los días lunes o martes, así tendremos más tiempo para comentar estos pequeños ejercicios.

Esta vez, se trata de dos páginas: la primera que se encarga de realizar el login de los usuarios y la segunda se encarga de mostrar y actualizar el perfil del usuario que accede a la página.

php:
<?php
session_start();

/*
 session.php:
  Se encarga de validar los datos del usuario y
  que exista una sesión válida para acceder a user.php
*/


include_once './config.php';
include_once './db.php';

if ( !empty($_REQUEST['action']) ) {
        switch ($_REQUEST['action']) {
                case 'login':
                        if ( !empty($_SESSION['user']) ) {
                                header('Location: http://sitio/user.php');
                                exit();
                        }
                       
                        $username = $db->escape($_POST['user']);
                        $pass = md5($secret_key . $_POST['password']);
                       
                        if ( $user = $db->get_row("SELECT * FROM users WHERE user='$username' AND password='$pass'") ) {
                                session_regenerate_id();
                                $_SESSION['user'] = $user;
                                header('Location: http://sitio/user.php');
                                exit();
                        }
                        die('Usuario no válido');
                case 'logout':
                        // ...
        }
}

?>
php:
<?php
/*
 user.php:
   Se encarga de mostrar y actualizar el perfil de un usuario
*/


include_once './session.php';

if ( empty($_SESSION['user']) ) {
        header('Location: http://sitio/session.php');
        exit();
}

if ( !empty($_REQUEST['action']) ) {
        switch ($_REQUEST['action']) {
                case 'update-profile':
                       
                        // Limpiar los datos
                        $_POST['name'] = htmlspecialchars(strip_tags($_POST['name']), ENT_QUOTES, 'utf-8');
                        $_POST['email'] = preg_replace('/[^a-z0-9.@-]/i', '', $_POST['email']);   

                        // Escapar los valores
                        $user = $db->escape($_SESSION['user']->user);
                        $name = $db->escape($_POST['name']);
                        $email = $db->escape($_POST['email']);
                        $password = md5($secret_key . $_POST['password']);
                       
                        $sql = "UPDATE users SET name = '%s', email = '%s', password = '%s' WHERE user = '%s'";
                       
                        $db->query(sprintf($sql, $name, $email, $password, $user));
                       
                        break;
                case 'recover-password':
                        // ...
        }
}

?>

¿Existe algún bug en el código mostrado?

Actualización:

  1. Cambio de $_POST['user'] por $_SESSION['user']->user en user.php
  2. Cambio de ubicación de la comprobación de la variable user en la sesión

¿Todavía queda algún problema de seguridad?

25 Replies to “Ejercicio de la Semana: Login de usuarios y actualización de datos”

  1. Yo diría que lo mismo que escapas la varible "$user" para usarla en sendas consultas SQL deberías también escapar la variable "$password". Vaya, no creo que sea este el "bug", porque sería acertar y tengo últimamente una suerte... 😀

  2. No importa escapar el valor de $password, puesto que este valor siempre será una cadena de 32 caracteres alfanuméricos (gracias a md5).

  3. alter, no, lo del password no es una vulnerabilidad ya que al aplicarle un md5 el resultado consiste solo en letras y numeros.

    Yo por mi parte no he encontrado ningun bug, solo un error de información que no creo fuera tu objetivo que se encontrase ese error lo encuentro cuando procedes a validar el login cuando llegas a esta porción de código:

    code:

    if ( $user = $db->get_row("SELECT * FROM users WHERE user='$username' AND password='$pass'") ) {
             session_regenerate_id();
             $_SESSION['user'] = $user;
    }
    die('Usuario no válido');
     

    ya que session_regerate_id no refresca la página, aunque el usuario se válido das el mensaje usuario no válido.

  4. no se si sera un bug pero al modificar los datos, sobre todo por lo de modificar la contraseña, no pides la contraseña antigua, un usuario maligno podria iniciar session, y despues manipular el html de su navegador para cambiar el valor del campo user para cambiar la contraseña de otro usuario, creo que existe alguna extension en firefox que permite modificar el formulario.

    En caso de que no se pueda usar el navegador podrias crear una url con los mismos campos del formulario ya que el cambiar de pagina en la misma ventana del navegador no expira la session. No compruebas el referrer para averiguar si el envio del formulario proviene de la misma pagina.

    Pienso que los tiros van por ahi.

  5. Cambiando

    php:

    $user = $db->escape($_POST['user']);

    //sustituir por

    $user = $db->escape($_SESSION['user']->user);
     

    Se soluciona el problema, ya solo se pueden cambiar los datos que te corresponden y ya no puedes cambiar el nombre del usuario. Ya no seria necesario pedir la clave antigua, aunque seria un buen mecanismo para evitar que el usuario pusiese accidentalmente una clave nueva, ademas, no compruebas si la pass es vacia, lo cual cambias siempre la clave.

    pongo mi version sin confirmacion de la clave antigua

    php:

    escape($_SESSION['user']); //solo permitimos modificar el usuario logueado
                            $name = $db->escape($_POST['name']);
                            $email = $db->escape($_POST['email']);
                            $_POST['password']==''?$password='':$password = md5($secret_key . $_POST['password']);
                            //me gusta inicializar variables sobre todo cuando son criticas
                           
                            $sql = "UPDATE users SET name = '%s', email = '%s'";
                            if($password!='')
                                    $sql.=', password = '%s'
                            $sql.='
    WHERE user = '%s'";
                           
                            if($password=='')
                                    $db->query(sprintf($sql, $name, $email, $user));
                            else
                                    $db->query(sprintf($sql, $name, $email, $password, $user));
                           
                            break;
                    case 'recover-password':
                            // ...
            }
    }
     
    ?>
  6. $sql.=', password = '%s' me equivoke

    la comilla que abre la cadena deberia ser doble, y deberia cerrar la cadena y poner ;

    y en $sql.=' WHERE ... la comilla que abre la cadena deberia ser doble

    queda algo feo el codigo todo rojo voy a ponerlo otra vez

    php:

    escape($_SESSION['user']); //solo permitimos modificar el usuario logueado
                            $name = $db->escape($_POST['name']);
                            $email = $db->escape($_POST['email']);
                            $_POST['password']==''?$password='':$password = md5($secret_key . $_POST['password']);
                            //me gusta inicializar variables sobre todo cuando son criticas
                           
                            $sql = "UPDATE users SET name = '%s', email = '%s'";
                            if($password!='')
                                    $sql.=", password = '%s'";
                            $sql.=" WHERE user = '%s'";
                           
                            if($password=='')
                                    $db->query(sprintf($sql, $name, $email, $user));
                            else
                                    $db->query(sprintf($sql, $name, $email, $password, $user));
                           
                            break;
                    case 'recover-password':
                            // ...
            }
    }
     
    ?>
     
  7. copie y pegue el codigo pero me corto el trozo de arriba el cual permaneceria igual

    y donde pone

    $user = $db->escape($_SESSION['user']);

    habria que poner

    $user = $db->escape($_SESSION['user']->user);

    Pido disculpas por tantos posts y tantas erratas por mi parte.

  8. jdeveloper, en el comentario #6 tienes razón, pero cuál sería la forma de cambiar el password sin necesidad de acceder o tener las credenciales del ejemplo mostrado.

  9. Voy a escribir una tonteria pero bueno.

    Suponiendo que las sessiones se manejan por cockies, tambien se pueden conifgurar para que funcione exclusivamente de lado del servidor si mal no recuerdo, el procedimiento seria hacer lo siguiente:

    1) crear la cookie user en el navegador que seguro que hai forma
    2) mandar el post a user.php para modificar.

    Pero eso queriria decir que en la mayoria de las paginas en internet seria posible asi que casi que descarto la idea.

    Un saludo.

  10. Hola!

    Mi amigo jdeveloper y yo hemos estado rayando profundamente sobre el tema, llegamos a la conclusión de que un posible error esta en el basename($_SERVER['PHP_SELF']) del archivo session.php ya que podríamos burlar la condición llamando a una url del tipo http://sitio/user.php/session.php. Esto nos retornaria el basename session.php, con lo cual podríamos modificar nuestros datos sin loguearnos previamente.

    Por ahora, que seguro que más mal se pude hacer es todo.

    Saludos.

  11. ¿Se supono que tenemos usuario en el sistema?
    ¿El objetivo es entrar como otro usuario?

    Si salimos de la premisa que tenemos un usuario y el objetivo es entrar como otro.

    1) Entraremos en nuestro usuario.
    2) Y enviamos tipo POST a la pagina de registro las siguientes variables:
    user = usuario_victima
    password = el_que_queramos
    3) entramos de nuevo como usuario_victima

    (la solución la ha dado jdeveloper)

  12. Agustí, tienes razón -no me había dado cuenta de ese detalle :), ya hice las correcciones planteadas por jdeveloper (excepto lo de la comprobación del password vacío)

  13. Perdón salieron los 3 mal, si este sale mal, please alex arreglalo... os envio el "exploit" del bug...

    en #14 ya han dado la pista definitva, yo no me habia fijado en que el fichero session.php es el primero.... así que el include nos obliga a hacer la llamada al la pagina user.php tal y como dicen ellos...

    html:

    <html>
    <body>
    <form action="http://sitio/user.php/session.php" method=POST>
    user: <input name="user" /><br/>
    pass: <input name="password" /> <br/>
    email: <input name="email" /> <br/>
    name: <input name="name" /> <br/>

    <input name="action" value="update-profile" type="hidden" />
    <input type="submit" />
    </form>
    </body>
    </html>
     

  14. ALGUIEN E PODRIA DECIR COMO AVERIGUAR LA CONTRASENA DE UN EMAIL????

    ESCRIBANME A :gimtaras55@gmail.COM

    ADEMAS YO TENGO ACCESO A ESE ORDENADOR: NECESITO DE UN EMAIL, Y DE SKIPE

  15. copie el copie el codigo del formulario y me salio este error

    Warning: session_start() [function.session-start]: Cannot send session cookie - headers already sent by (output started at D:\AppServ\www\siscotizacion\user.php:5) in D:\AppServ\www\siscotizacion\user.php on line 6

    Warning: session_start() [function.session-start]: Cannot send session cache limiter - headers already sent (output started at D:\AppServ\www\siscotizacion\user.php:5) in D:\AppServ\www\siscotizacion\user.php on line 6

    alguien me podria ayudar en verdad esta muy interesante el tema pero podrian ayudarme
    si alguien tuviera el codigo o el proyecto podria mandarmelo desde ya gracias mi correo es sdarknot@gmail.com

Comments are closed.