[Resuelto] SQLi en esta consulta?

[#Aitor]

Tenía la duda de si se puede inyectar código a esta consulta desde los formularios usuario/contraseña.

Código (php) [Seleccionar] Expandir



/*________________________________________
         MÉTODO INICIAR SESIÓN
 ________________________________________*/
   public function Iniciar_sesion($usuario, $password){
     session_start();

     if(isset($_POST['user']) && strlen($_POST['user'])>0 &&
     isset($_POST['password']) && strlen($_POST['password'])>0) {

       $Save_password = $this->connection->query("SELECT PASSWORD FROM usuarios WHERE USER = '$usuario' ")->fetchColumn();

      if($Save_password === md5($password)){
           $_SESSION['user'] = $this->connection->query("SELECT USER FROM usuarios WHERE USER = '$usuario'")->fetchColumn();

           # Si los datos introducidos son correctos.
      header('Location:'. $_SERVER["HTTP_REFERER"]);

           # Si la contraseña es incorrecta.
      }else{header("Location:Login.php");}

         # Si los campos están vacios.
     }else{header("Location:index.php");}
   }



Diría que sí, ya que en el formulario usuario se podría inyectar perfectamente código para la consulta que se genera.

¿Cómo se podría solucionar?

[#!drvy]

Si, es vulnerable a SQLi.

Solución: Usa prepared statements.

http://php.net/manual/es/mysqli.quickstart.prepared-statements.php
http://php.net/manual/es/pdo.prepare.php

No uses MD5 para contraseñas.. hoy en día es casi inútil, una GPU decente te puede hacer millones de comparaciones de hash por segundo lo cual lo hace super viable e inseguro a ataques de fuerza bruta.

Usa cifrados con factor de dificultad:

http://php.net/manual/es/function.password-hash.php
http://php.net/manual/es/function.password-verify.php
http://php.net/manual/es/function.crypt.php

Saludos

[#Aitor]

Si, es vulnerable a SQLi.

Solución: Usa prepared statements.

http://php.net/manual/es/mysqli.quickstart.prepared-statements.php
http://php.net/manual/es/pdo.prepare.php

No uses MD5 para contraseñas.. hoy en día es casi inútil, una GPU decente te puede hacer millones de comparaciones de hash por segundo lo cual lo hace super viable e inseguro a ataques de fuerza bruta.

Usa cifrados con factor de dificultad:

http://php.net/manual/es/function.password-hash.php
http://php.net/manual/es/function.password-verify.php
http://php.net/manual/es/function.crypt.php

Saludos

¡Genial, muchas gracias!

Y en cuánto a consultas estáticas ¿me recomiendas prepararlas y ejecutarlas o dejarlas simplemente como están? ¿supongo que dejarlas como están porque hacer una consulta genera menos consumo que prepararla y ejecutarla? Aunque quizá sea una buena práctica empezar a preparar todas las consultas...

Gracias Dryv!

[#!drvy]

Yo generalmente, cuando no uso un ORM (que suelo usar casi todo el rato, y te lo recomiendo), lo que me hago es una función de query.. dicha función acepta mínimo dos parámetros, la query (obligatorio) y un array (opcional). Si el array viene con datos, ejecuto una sentencia preparada, si el array esta vació, hago un "exec".

Aunque realmente, creo que la diferencia entre una sentencia preparada y una directa en cuanto a eficiencia es mínima.

Saludos

[WHK]

Para eso existe: http://php.net/manual/es/mysqli.real-escape-string.php

[#!drvy]

Nota: Es 100 veces más seguro usar sentencias preparadas que "escapar" datos.

Comparalo con una frontera.. ¿prefieres "procesar" a cada visitante extranjero o directamente meterle en un área aislado?

https://ilia.ws/archives/103-mysql_real_escape_string-versus-Prepared-Statements.html

Saludos

[#Aitor]

Para eso existe: http://php.net/manual/es/mysqli.real-escape-string.php

Había visto esa opción pero no me apetecía usar Mysqli, ya que todo lo había empezado con PDO, que desde mi punto de vista es algo mejor, además la idea de escapar caracteres tampoco me llamaba mucho la atención, porque imagino que al igual que con las inyeciones XSS, habrá formas de saltarse ese escape.

lo que me hago es una función de query.. dicha función acepta mínimo dos parámetros, la query (obligatorio) y un array (opcional). Si el array viene con datos, ejecuto una sentencia preparada, si el array esta vació, hago un "exec".

Al final termine haciendo un método privado para realizar las consultas preparadas/simples en función de si el array era null o no.

Gracias, como siempre!

[WHK]

Lo peor que puedes hacer es usar pdo para mysql para eso existe la extensión nativa mysqlnd para php con el cual puedes manejar mysql con mysqli::, no creas mucho que puedes migrar de un motor a otro con pdo porque de todas maneras necesitarás cambar sintaxis y otras cosas importantes.

Hay varias maneras de crear consultas sql, preparadas, escapadas, persistencia modelada, etc, pero no quiere decir que utilizando una manera tu proyecto será mas o menos inseguro, el creador de php creó todas las funciones necesarias para prevenir las inyecciones entre otras cosas para cada tipo de manipulación de bases de datos asi que en la práctica estarás seguro aun utilizando consultas a modo de texto plano.

Lo que si, es mucho mas recomendable utilizar modelamiento de datos y consultas parametrizadas por temas de seguridad en la escalabilidad, esto quiere decir que si el dia de mañana tu desarrollo es tomado por otra persona correrás el riesgo de que caiga en crear un agujero de tipo inyección sql sin no sabe utilizar bien las funciones de php creadas para que eso no pase, en cambio las consultas preparadas te dan la ventaja de que muy dificilmente una persona tendrá inyección sql debido a que ya no necesita realizar los escapes de manera manual.

Por otro lado te recomiendo utilizar codeigniter ya que estás utilizando php, codeigniter ofrece algo llamado Active Record el cual son sentencias basadas en objetos, ya no necesitas parametrizar ni escapar cosas, es todo mas fluido y viene nativamente con varios drivers para conectarse a diferentes tipos de bases de datos sin la necesidad de modificar tus consultas sql.

Si piensas hacer un proyecto grande y que sea mantenible con el tiempo (transaccional/escalable) te recomiendo que comiences por aprender codeigniter y migres tu proyecto a algo con mejores prácticas.

https://www.codeigniter.com/userguide2/database/active_record.html
https://www.codeigniter.com/userguide3/database/query_builder.html

[#!drvy]

Lo peor que puedes hacer es usar pdo para mysql para eso existe la extensión nativa mysqlnd para php con el cual puedes manejar mysql con mysqli::, no creas mucho que puedes migrar de un motor a otro con pdo porque de todas maneras necesitarás cambar sintaxis y otras cosas importantes.

Es que PDO usa el driver de mysql... Pruebalo, intenta usar PDO sin haber instalado el modulo php-mysql.. Por mucho que necesites cambiar de sintaxis, usando PDO NO necesitaras cambiar de funciones.. todo tiene el mismo procedimiento.. Por no hablar de otras ventajas que ofrece PDO como los parámetros nombrables.. Compara esto:

Código (php) [Seleccionar] Expandir

SELECT nombre, apellidos, direccion FROM clientes WHERE nombre = :nombre AND apellidos = :apellidos

Contra esto:

Código (php) [Seleccionar] Expandir

SELECT nombre, apellidos, direccion FROM clientes WHERE nombre = ? AND apellidos = ?

..

Lo del ORM (que es básicamente lo que mencionas) si es recomendable, tal y como recomendé en un post anterior.

Saludos