bug encontrado

0 views
Skip to first unread message

shadow

unread,
May 25, 2009, 4:05:54 PM5/25/09
to Yupp Framework PHP
Saludos a todos los integrantes del grupo

En dias anteriores estaba haciendo pruebas con el modulo de login y
detecte el siguiente error:

El error esta en la parte del password Condiction::EQ(tabla, atributo,
dato_a_comparar)
lo que descubri fue lo siguiente: si en la base de datos tienes como
valor para el password "Hola" y en el formulario del login pones
"hola" te da acceso a la pagina, lo cual no deberia de pasar

lo que hice para solucionar este problema fue utilizar la funcion
strcmp() quedando el codigo asi:

public function loginAction()
{
if ( array_key_exists( 'doit', $this->params) )
{
if (!array_key_exists('usuario',$this->params) || !
array_key_exists('password', $this->params))
{
$this->flash['message'] = "Por favor ingrese nombre de
usuario y password";
return $this->render("/usuario/login", &$this->params);
}

$tableName = YuppConventions::tableName( 'Usuario' );
$condition = Condition::_AND()
->add( Condition::EQ($tableName, "usuario", $this-
>params['usuario']) )
->add( Condition::EQ($tableName, "password", $this-
>params['password']) );

$list = Usuario::findBy( $condition, &$this->params );

if ( count($list) === 0 )
{
$this->flash['message'] = "Nombre usuario o contrasña
incorrecta";
return $this->render("/usuario/login", &$this->params);
}

//SOLUCION AL PROBLEMA
if (strcmp($list[0]->getPassword(),$this->params
['password'])!=0)
{
$this->flash['message'] = "La contraseña es
incorrecta";
return $this->render("/usuario/login", &$this->params);
}
//FIN DE LA SOLUCION

YuppSession::set("user", $list[0]);

$this->flash['message'] = "Bienvenido ".$list[0]->getNombre
();
return $this->redirect( array("controller" => "usuario",
"action" => "list") );
}
return $this->render("/usuario/login", &$this->params);
}

Espero que este aporte les ayude en algo...

Saludos...

pablopazos

unread,
May 25, 2009, 4:12:03 PM5/25/09
to Yupp Framework PHP
Excelente aporte Ivan!

Como comentario: la consulta generada es la siguiente:

SELECT * FROM usuarios WHERE (usuarios.class='Usuario' AND
usuarios.deleted=0 AND (usuarios.email='pabl...@gmail.com' AND
usuarios.clave='hola')) ORDER BY id asc LIMIT 10 OFFSET 0

Se ve que el "=" no funciona como uno esperaría para strings en MySQL,
seguramente lo deba cambiar el generador de la consulta, ya que afecta
también a todas las otras consultas que se hagan. Busqué en la
referencia de MySQL y lo que habría que usar en la consulta es la
función STRCMP de MySQL, ya que el "=" no considera mayúsculas y
minúsculas ni tampoco tildes (e=é).

Voy a agregar la solución de este bug para la versión 0.1.6.5,
mientras agregaré tu solución al código del blog de ejemplo que viene
con Yupp (esto saldrá en la versión 0.1.6.4 que está próxima a ser
liberada luego de algunas pruebas que faltan hacer).


Muchas gracias por tu aporte!

Saludos,
Pablo.

Tio Oscar

unread,
May 25, 2009, 6:33:28 PM5/25/09
to yuppfram...@googlegroups.com
Lo mejor es mandarle BINARY a la query en todo caso:


SELECT * FROM usuarios WHERE (usuarios.class='Usuario' AND
usuarios.deleted=0 AND (usuarios.email='pabl...@gmail.com' AND
BINARY usuarios.clave='hola')) ORDER BY id asc LIMIT 10 OFFSET 0

Pero igual, y perdon si sueno agresivo, pero ¡¡¡¡es de inconciente guardar una contraseña en texto plano!!!!

Las contraseñas nunca se guardan literales, en todo caso se genera una hash y se guarda ese hash:

hola :

MD5: 4d186321c1a7f0f354b297e8914ab240
SHA1: 99800b85d3383e3a2fb45eb7d0066a4879a9dad0
MD5(MD5): cf96bce69f409820e4b6bce661eb4e78

Hola :

MD5: f688ae26e9cfa3ba6235477831d5122e
SHA1: 4e46dc0969e6621f2d61d2228e3cd91b75cd9edc
MD5(MD5): 39a8af994cd9cde9f93f7cba01a5e464

El MD5 es un string de 32 caracteres (BINARY varchar(32)) y el SHA1 de 40 caracteres (BINARY varchar(40)).

Aca dejo doc:

http://ar2.php.net/md5
http://ar2.php.net/manual/es/function.sha1.php
http://ar2.php.net/crypt
--
El Tio ~ Programador, hacker y filósofo
Blog: http://blog.exodica.com.ar
Linked'in: http://www.linkedin.com/in/ogentilezza
Tel: [+54 11] 638-LINUX (584689)
Movil: [+54 9 11] 5586-4955

No al canon digital en Argentina! www.noalcanon.org

pablopazos

unread,
May 25, 2009, 6:47:16 PM5/25/09
to Yupp Framework PHP
Buenas!

El problema no es solo de esa consulta, el problema está en cómo el
framework genera las consultas para MySQL y que MySQL no compara
strings como uno esperaría al usar "=". O sea, el problema no se
resuelve cambiando el tipo de dato, ya que en otros casos donde se
necesite un string, el error serguirá existiendo. (esto está en el
paquete db.criteria, en la clase Condition).

Lo de la contraseña estoy de acuerdo (creo que ya lo comentamos en
otra oportunidad), recordemos que el blog que viene implementado con
Yupp es una aplicación "de muestra" y está lejos de ser una aplicación
para producción. Eso si, Ivan, si vas a usar el login para algún
proyecto te recomiendo que sigas los consejos de Oscar.


Muchas gracias por sus aportes!

Saludos,
Pablo.

On 25 mayo, 19:33, Tio Oscar <tios...@gmail.com> wrote:
> Lo mejor es mandarle BINARY a la query en todo caso:
>
> SELECT * FROM usuarios WHERE (usuarios.class='Usuario' AND
> usuarios.deleted=0 AND (usuarios.email='pablo....@gmail.com' AND
> *BINARY* usuarios.clave='hola')) ORDER BY id asc LIMIT 10 OFFSET 0
>
> Pero igual, y perdon si sueno agresivo, pero ¡¡¡¡es de inconciente guardar
> una contraseña en texto plano!!!!
>
> Las contraseñas nunca se guardan literales, en todo caso se genera una hash
> y se guarda ese hash:
>
> hola :
>
> MD5: 4d186321c1a7f0f354b297e8914ab240
> SHA1: 99800b85d3383e3a2fb45eb7d0066a4879a9dad0
> MD5(MD5): cf96bce69f409820e4b6bce661eb4e78
>
> Hola :
>
> MD5: f688ae26e9cfa3ba6235477831d5122e
> SHA1: 4e46dc0969e6621f2d61d2228e3cd91b75cd9edc
> MD5(MD5): 39a8af994cd9cde9f93f7cba01a5e464
>
> El MD5 es un string de 32 caracteres (BINARY varchar(32)) y el SHA1 de 40
> caracteres (BINARY varchar(40)).
>
> Aca dejo doc:
>
> http://ar2.php.net/md5http://ar2.php.net/manual/es/function.sha1.phphttp://ar2.php.net/crypt
>
> El 25 de mayo de 2009 20:12, pablopazos <pablo....@gmail.com> escribió:
>
>
>
>
>
> > Excelente aporte Ivan!
>
> > Como comentario: la consulta generada es la siguiente:
>
> > SELECT * FROM usuarios WHERE (usuarios.class='Usuario' AND
> > usuarios.deleted=0 AND (usuarios.email='pablo....@gmail.com' AND

shadow

unread,
May 25, 2009, 6:52:48 PM5/25/09
to Yupp Framework PHP
No te preocupes si suena agresivo

Tienes razon en lo de enviar texto plano, pero he leido que las
funciones MD5, SHA1 son unicamente funciones de encriptacion, en mi
caso voy a a estar enviando los datos por medio de HTTPS porque
necesito enviar las contraseñas al correo del usuario cuando este lo
solicite.

ahorita estoy bucando algun metodo para enviar informacion sin
necesidad de comprar un certificado ssl en caso de que se pueda.

igual si tienes alguna sugerencia, ya que yo no tengo mucha
experiencia, soy recienegresado y estoy realizando mis primeros
proyectos.

saludos

Tio Oscar

unread,
May 25, 2009, 7:09:43 PM5/25/09
to yuppfram...@googlegroups.com
Podés enviar la contraseña encriptada usando una semilla que sepa solo el server que lo envia y el que lo recibe, eso lo podes hacer con crypt y funciones propias, o podes usar directamente la extensión crypt.

y lo de enviarle la contrasñe al usuario es algo que se dejo de usar justamente cuando se tomo conciencia de que un simple error xss o sq injection o el hecho de que alguien simplemente vea esa tabla deja las contraseñas de los usuarios al descubierto, loq ue se hace hoy por hoy es generar una contraseña nueva, enviarla por correo, y en todo caso una vez logueado ofrecerle que la cambie por alguna mas comoda para el usuario.

MM no se donde lo vi, creo que en la web de php (en los comentarios de la documentacion) 2 funciones para encriptar/desencriptar datos usando una cemilla.

pablopazos

unread,
May 25, 2009, 10:06:55 PM5/25/09
to Yupp Framework PHP
Shadow, agregué tu parche al framework, ya está disponible en el SVN
del Google Code del proyecto (http://code.google.com/p/yupp/) y saldrá
con la versión 0.1.6.4 del framework:

http://code.google.com/p/yupp/source/browse/
http://code.google.com/p/yupp/source/browse/YuppPHPFramework/components/blog/controllers/components.blog.controllers.UsuarioController.class.php


Saludos,
Pablo.

On 25 mayo, 17:05, shadow <idesantiagocontre...@gmail.com> wrote:

pablopazos

unread,
Jun 15, 2009, 11:56:24 PM6/15/09
to Yupp Framework PHP
Agregué la corrección al framework, ahora "Condition" tiene una nueva
condición: STREQ, que respeta mayúsculas y minúsculas, y usa
exactamente lo que dijo Oscar (el tema de BINARY en MySQL).

El problema es que esta solución queda atada a MySQL. Por ejemplo, en
SQLite este problema no pasa por que los operadores son case-
sensitive, por lo que será necesario modificar el módulo de generación
de consultas para que pueda adaptarse a cualquier DBMS sin problemas
como estos, y que las sutilezas de cada uno se resuelvan internamente.

Saludos,
Pablo.

PD: la solución y el login corregido del módulo "blog" ya están en el
repositorio SVN: http://code.google.com/p/yupp/source/checkout

On 25 mayo, 23:06, pablopazos <pablo....@gmail.com> wrote:
> Shadow, agregué tu parche al framework, ya está disponible en el SVN
> del Google Code del proyecto (http://code.google.com/p/yupp/) y saldrá
> con la versión 0.1.6.4 del framework:
>
> http://code.google.com/p/yupp/source/browse/http://code.google.com/p/yupp/source/browse/YuppPHPFramework/componen...
Reply all
Reply to author
Forward
0 new messages