Cómo utilizar PHP Code Sniffer para revisiones de código

En Human Made nos llegan muchísimos Pull Requests en todos los proyectos que tenemos de forma diaria y por regla todos los PRs tienen que ser revisados. Si quieres aprender bien en qué consiste una revisión de código y cómo llevarla a buen término, recomiendo estos dos posts: https://mtlynch.io/human-code-reviews-1/https://mtlynch.io/human-code-reviews-2/. Por cierto, si en tu lugar de trabajo no haceis revisiones de código, nunca es tarde para empezar. Se aprende muchísimo y dejas de tomarte muchas cosas como algo personal. Además aprendes a tratar con las personas de tu equipo y lo mejor es que el código de producción mejorará notablemente.

Hacer una revisión de código sobre grandes cambios es algo muy difícil. Aparte de lo que conlleva en cuanto a la delicadeza de decirle a alguien que sus cambios son una birria o que no valen (tratar con gente es lo más difícil aquí por si no te habías dado cuenta, estimado lector); GitHub no ofrece una buena herramienta para revisar código: No tenemos colorines o no hay nada que nos señale dónde puede estar el error. De forma adicional, si estamos siguiendo un coding standard (¿A qué estás esperando?), buscar esos errores es muy pesado y aporta bien poco a ambas partes.

Lo ideal es que en nuestro proyecto tengamos ya instaladas herramientas para ahorrarnos las revisiones de sintaxis como PHP Code Sniffer, ESLint o JSHint (Aquí tengo un repositorio para empezar un proyecto en WordPress con todo eso: https://github.com/igmoweb/wp-phpcs-boilerplate) pero no siempre es así y puede que necesitemos hacer algún tipo de revisión en un repositorio en el que no hay integración contínua. Para ahorrarme tarea suelo utilizar phpcs instalado globalmente y así escanear un directorio completo. Para eso hay que seguir una serie de pasos:

Instalar phpcs

Hay que instalarlo de forma global en nuestro equipo usando Composer (también instalado de forma global):

composer global require squizlabs/php_codesniffer

Si quisiéramos ejecutar phpcs, tendríamos que lanzar el siguiente comando:

~/.composer/vendor/bin/phpcs test.php

Pero sería mejor si accediéramos a phpcs sin escribir toda la ruta. Para ello podemos crearnos un enlace simbólico dentro de nuestra carpeta personal ~/bin (Si no existe, puedes crearla).

ln -s ~/.composer/vendor/bin/phpcs ~/bin/phpcs

Seguramente tengas que reiniciar la consola para que el cambio funcione. Para probar puedes ejecutar phpcs sobre cualquier fichero PHP:

phpcs test.php

 

Instalar reglas de validación

Con PHP Code Sniffer puedes indicar las reglas que te gustaría aplicar sobre tu código mediante un fichero de configuración xml. Aquí vamos a usar un conjunto de reglas específicas para cumplir el estándar de WordPress además de la librería PSR2R que contiene un monton de reglas más.

composer global require wp-coding-standards/wpcs
composer global require fig-r/psr2r-sniffer

Ya tenemos todo lo necesario para empezar a trabajar

 

Configuración de phpcs

Ahora simplemente tenemos que indicar las reglas que vamos a utilizar. Para eso vamos a crear un fichero .xml. ¿Dónde? Donde quieras en tu equipo, en alguna carpeta que localices fácilmente, yo suelo usar ~/source para meter todo mi código.

Vamos a crear dos ficheros:

  • phpcs-security.xml: Sólo para las reglas más básicas de seguridad.

Aquí usamos varias reglas de seguridad como revisión de posibles problemas de XSS, CSRF y reglas de WordPress VIP sobre validación entre otras cuantas muy básicas que nos valen para hacer un primer chequeo de seguridad en cualquier código. Ten en cuenta que algunas rutas tienen que ser absolutas o al menos no he encontrado forma de usar ~/ para usar rutas relativas a nuestra carpeta home.

  • phpcs-all.xmlPara todas las reglas (cuidado, esto es lo más parecido a un compañero psicópata pero al menos puedes quitarle reglas).

En este ejemplo he eliminado una regla de las que vienen incluídas en el paquete de WordPress: Las Yoda Conditions y además he incluído otra de PSR2R: NoInlineAssignmentSniff.

Con esto ya tenemos todo listo para empezar a revisar código.

Revisar el código

Si algún compañero ha hecho un Pull Request en el repositorio de nuestro proyecto, podemos descargar la rama de Git en dicho proyecto:

cd mi-proyecto && git fetch origin pull-request-branch && git checkout pull-request-branch

Y ahora podemos ejecutar phpcs a todo el directorio.

phpcs --standard=/ruta/a/fichero/phpcs-security.xml

o también

phpcs --standard=/ruta/a/fichero/phpcs-all.xml

Con esto sería bastante sencillo crearnos un script que nos ejecute automáticamente el escaneo, incluso podríamos elegir qué fichero queremos utilizar:

Si copias ese fichero en tu carpeta bin (~/source/bin) y le das permisos de ejecución (sudo chmod +x ~/bin/review.sh) podrás escanear cualquier carpeta simplemente invocando review.sh desde dicha carpeta.

Con esto tenemos ya cubierto una buena parte en una revisión de código. Evidentemente, no hemos revisado todavía la lógica del código pero al menos podemos tener bastante claro si hay algún problema de seguridad con este método y todo en unos pocos segundos.

Nota: El script se podría mejorar un poco sacando el listado de archivos de un commit en concreto a partir del hash de Git. De esta forma no tendríamos que hacer un ecaneo a la carpeta completa (que puede ser enorme). Sería interesante verlo en funcionamiento.

Cross-site Request Forgery: Dos ejemplos para entenderlo

Cross-site Request Forgery consiste en la falsificación de petición en sitios cruzados.

Ajá

¿En qué consiste?

Cross-site Request Forgery o CSRF a partir de ahora, es un tipo de exploit difícil de conseguir llevar a buen término pero no exento de riesgos, y muy altos. Voy a saltarme definiciones formales y pasar directamente a un ejemplo porque es como mejor se entiende.

En el siguiente ejemplo yo voy a ser el hacker y el objetivo es el sitio web de un despreocupado usuario que se va a llamar Jaimito. Para llevar a buen término mi jugada, necesitamos cumplir ciertos requisitos previos:

  • Conocer previamente que Jaimito tiene en su web, http://www.soyjaimito.com, código susceptible a ser explotado mediante esta técnica. Por ejemplo, un plugin o un tema.
  • Que Jaimito haya hecho login y tenga permisos suficientes (aunque muchas veces no se necesitan siquiera dichos permisos).
  • Yo, genio cutre del mal, me voy a crear otra web con un simple formulario en otro sitio, llamémosle vas-a-ver.com. Dicho formulario provocará una acción que acabará en desastre para Jaimito.
  • Seguidamente, necesitaré atraer la atención de Jaimito a mi nueva y flamante web. No es muy difícil conseguir un email de contacto de la web y enviarle uno con un link a ésta.
  • Necesitamos que Jaimito sea un poco inocentón.

El código explotable

Vamos a imaginar que en WordPress.org hay un plugin que contiene una vulnerabilidad de tipo CSRF y que Jaimito lo tiene instalado en http://www.soyjaimito.com. Vamos a suponer que el plugin no hace nada de nada pero tiene el siguiente código:

Es muy fácil: El plugin borra un usuario si $_POST['action'] === 'delete-user' y el usuario tiene suficientes permisos (en este caso, un administrador).

Aparentemente no hay problema, sólo los administradores pueden borrar el usuario que indiquen. Pero yo sé que no es así, ahora veremos porqué.

Me voy a crear mi sitio, vas-a-ver.com, que tiene el siguiente código, un sencillísimo formulario HTML, ni PHP ni nada:

Y una vista de la web:

Captura de un simple formulario que pone

Es un formulario que cuando se procesa redirige a http://www.soyjaimito.com y envía mediante un request de tipo POST dos datos:

  • user_id = 1
  • action = delete-user

Creo que ya se empieza a ver el objetivo de mi página pero si yo hiciera click sobre el botón que anuncia un regalo por la cara, no valdría de nada porque el sitio de Jaimito comprueba si tengo permisos o no. Lo que hay que hacer es atraer a Jaimito a mi web.

Como ya hemos dicho, esto no es muy difícil y además Jaimito es un poco inocente y va a picar sí o sí. En un caso más sofisticado crearíamos una web que se parezca a otra en la que Jaimito confíe, como Apple, Google o Facebook. Lleva más tiempo pero no es demasiado difícil replicar los estilos de páginas de ese tipo.

Resulta que le envío un email a Jaimito con el asunto “¡Has ganado un iPhone, Jaime! ¡Enhorabuena! Pincha aquí”. Si escribo el email con un poco de esmero y sin faltas de ortografía, Jaimito pinchará en el enlace que le he mandado y que redirige a mi web. Seguidamente Jaimito hará click en el botón pensando que ha ganado un iPhone y si ha hecho login en su página http://www.soyjaimito.com (si la utiliza mucho, normalmente lo estará) se encontrará con su web de siempre pero con una diferencia: El usuario con ID=1 se ha borrado. El usuario 1 suele ser el administrador y, sí, en este caso es también el usuario de Jaimito. ¡Desastre!

¿Cómo prevenirlo?

El método de prevención es muy sencillo y si nos acostumbramos a usarlo en cualquier formulario sin excepción, no tendremos este problema.

El método trata de usar una cadena alfanumérica generada en nuestro propio sitio que sea aleatoria y que enviemos junto con el formulario. Al procesarlo comprobaremos si dicha cadena es correcta. Si lo es, quiere decir que el formulario se ha enviado desde nuestro sitio, en otro caso sabemos que no es así.

En WordPress utilizamos los nonces. No me voy a extender mucho en la explicación pero para generar un nonce dentro de un formulario usamos la función

<?php wp_nonce_field( $action, $name, $referer, $echo ) ?>

https://codex.wordpress.org/Function_Reference/wp_nonce_field

Dicha función genera algo así:

El HTML que saca la función wp_nonce_field()

Dos campos:

  • _wpnonce: Incluye un token o cadena alfanumérica que luego comprobaremos.
  • _wp_http_referer: La URL relativa desde la que enviaremos el formulario.

Dicho esto vamos a corregir el plugin para comprobar dicho nonce. Primero usaremos la siguiente línea en el formulario que se encarga de borrar usuarios. Éste no lo he incluído en el código del plugin pero se trata de un formulario normal y corriente (no confundir con el de vas-a-ver.com, en ese no podríamos generar el nonce porque si lo hiciéramos no generaríamos una cadena válida. Sólo nuestro sitio puede generarlo).

<?php wp_nonce_field( 'delete-user', 'delete-user-nonce' ) ?>

Y aquí el código del plugin corregido:

Si el formulario lo procesamos desde wp-admin podríamos usar la siguiente función para comprobar el nonce: https://codex.wordpress.org/Function_Reference/check_admin_referer pero en realidad no tiene importancia.

En resumen:

  • WordPress genera un token llamado nonce que es único.
  • El formulario envía el token junto con los datos.
  • Al procesar el formulario comprobamos que el nonce es válido.
  • Si el token no es válido o no existe sabemos que no ha sido enviado desde nuestro sitio y puede que nos hayan engañado. En otro caso, podemos procesarlo y borrar el usuario que nos señala el formulario en el campo user_id.

 

Otro ejemplo con más gracia

Recientemente encontré una vulnerabilidad de este tipo en un plugin del repositorio. No voy a decir el plugin y además lo arreglaron en unos pocos días. El caso es más interesante que el anterior porque requiere además un poco de pillería por parte del hacker.

El código con problemas era más o menos esto:

Es una llamada AJAX (un poco simplificado) que recoge los datos de un formulario y envía un email a support-desk@es-un-mal-plugin.com. Es decir, al servicio de soporte del plugin. Incluirá también en las cabeceras el usuario que haya hecho login, en este caso vuelve a ser el pobre Jaimito.

Y este sería el formulario desde vas-a-ver.com, es decir, el formulario que yo, genio cutre del mal, he creado.

A la vista es igual que el anterior pero los datos enviados son distintos y la URL destino es ahora http://www.soyjaimito.com/wp-admin/admin-ajax.php. Así hacemos una petición AJAX en WordPress.

Al procesar el formulario viene lo curioso. Con los valores del formulario que he creado, el email se mandará con las siguientes características:

  • From: tomará el valor “Jaimito <contact@soyjaimito.com>”
  • Reply-to: email-hacker@hacker.com
  • Texto: Hola, necesito ayuda con vuestro plugin. Se me ha roto, ¿Podríais enviarme mis datos privados ya de paso? Es que he perdido la contraseña
  • Asunto: Socorro

Y el email se envía al soporte del plugin. Si Jaimito además es un usuario registrado premium, es muy probable que los de soporte vean que el email viene de Jaimito (cabecera From) pero cuando le den a responder al email, me respondan a mí (cabecera Reply-To): email-hacker@hacker.com.

Y aquí entra la picaresca y sofisticación del hacker. Los de soporte pensarán que hablan con Jaimito pero nada más lejos de la realidad. Si soy un poco espabilado les puedo sacar contraseñas o datos privados a los de soporte y hacer cosas peores que borrar usuarios de un sitio.

Todo esto parece muy rebuscado…

Lo es. Para explotar un CSRF hay que cumplir muchas, muchas condiciones y que todo vaya bien paso a paso pero como resulte ir bien nos pueden crear un problema muy gordo.

Lecturas recomendadas

Algunos casos de grandes corporaciones que se han visto afectadas con un CSRF: http://freedom-to-tinker.com/2008/09/29/popular-websites-vulnerable-cross-site-request-forgery-attacks/

Essential PHP Security de Chris Shiflett [O’Reilly]. Explica con un lenguaje sencillo las típicas vulnerabilidades web y cómo prevenirlas en PHP. Muy recomendable.