Problemas de seguridad (SQL Injection) en el código

[clopez]

Hola,

He estado ojeando el código y existen decenas de problemas de seguridad del tipo SQL Injection.

Los parametros que se reciben del usuario no se pueden pasar directamente en la consulta SQL, ni directamente ni tras pasarlos por un simple trim().

Lo minimo necesario es usar alguna función como mysql_real_escape_string() sobre cualquier parametro que es pasado en una consulta SQL antes de construir la consulta.

No obstante aun con mysql_real_escape_string() puede haber casos en los que la aplicación siga siendo vulnerable a un ataque sql injection. La solución recomendada es usar consultas parametrizadas.

http://webappsec.org/projects/articles/091007.txt http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php

He analizado el código con RIPS (http://rips-scanner.sourceforge.net/) y esto es lo que ha reportado:

| Type of vulnerabily         | Number |
|-----------------------------|--------|
| Header Injection:           | 1      |
| File Disclosure:            | 2      |
| File Inclusion:             | 1      |
| File Manipulation:          | 4      |
| SQL Injection:              | 41     |
| Cross-Site Scripting:       | 8      |
| HTTP Response Splitting:    | 5      |
|-----------------------------|--------|
| Sum:                        | 62     |

[JavierGonzalez]

Hola Carlos.

En primer lugar agradecerte la molestia que te has tomado en analizar y reportar los bugs que has encontrado en VirtualPol.

En segundo lugar -a riesgo de que no sea de tu interés- me gustaría contarte mi situación personal y por consiguiente la del desarrollo. Se resume en que apenas tengo tiempo :) Estoy involucrado en un proyecto gordo ajeno a VirtualPol y apenas tengo tiempo para mejorar VirtualPol. Sin embargo continúo manteniendolo y lo haré siempre que me quede aliento.

Sobre el bug SQL inyection que comentas obviamente lo tenía pendiente de parchear con mayor firmeza. A falta de tiempo lo fui dejando pasar... hasta que hoy has dado el toque de alerta.

Me disgusta tener que estar a estas horas dandole caña (despues de 10 horas programando), pero aun así te estoy realmente agradecido por preocuparte en mejorar esto.

Solo te pido a cambio un poco de paciencia y -si te parece bien- que me pases futuros reportes de seguridad por email através de desarrollo@virtualpol.com Así como pedirte una nueva auditoria gratuita (gracias) para verificar que el tema está resuelto. Podrás publicarlo más adelante, pero si me das unos dias de ventaja te lo agradeceria, así me puedo encontrar un hueco mejor :)

En fin, esto es todo. He hecho un cambio en el codigo que está aplicado desde hace 20 minutos, minutos despues de leer tu email, el cual aun no he podido analizar con suficiente detenimiento, pero he ido al grano. Lo he hecho con todas las prisas imaginables, así que clemencia! :)

Un saludo y gracias por todo una vez mas!

[clopez]

Hola,

No pretendía causar estrés, prisas, ni nada parecido. Disculpa si fue así. Comprendo lo del tiempo limitado, la mayoría de nosotros desarrollamos software libre como hobbie. La próxima vez que vaya a reportar algún bug relativo a seguridad de la aplicación contactare contigo en privado antes. Gracias por la sugerencia.

Lo que me motivo a reportar esto fue simplemente que tras probar virtualpol ayer decidí echarle un vistazo rápido al código y me quede un poco intranquilo por ver como se pasaban las parámetros que el usuario enviaba directamente a la consulta SQL. Ej:

./public_html/source/c-r.php:   $result = mysql_query("SELECT ID, pais FROM users WHERE nick = '".$_GET['a']."' LIMIT 1", $link);

Sin embargo no me di cuenta de que estos parámetros eran previamente validados al incluir inc-login.php. Y la herramienta de análisis estático tampoco es capaz de detectar esto y sigue reportando vulnerabilidades cuando evidentemente ya están corregidos. Mi culpa por sacar conclusiones precipitadas. Con el commit a7a0958 tiene mucho mejor pinta.

Por ultimo aprovecho felicitarte por la aplicación. Tiene muy buena pinta y funciona francamente bien.

Un saludo!