Cómo validar correctamente la descarga de un archivo en php

Este post es debido a un correo que recibí preguntando como evitar el full path disclosure y el directory transversal desde un fichero php que realiza descargas. El error que plantearé a continuación es muy común en muchos sistemas web y ya he publicado varias webs vulnerables con este mismo problema. Corresponde a la forma de descarga tipo download.php?dir=ficheros/&file=test.pdf.

secure

Con una URL de este tipo, es posible forzar un FPD modificando las variables “dir” y “test”, cambiadolas por cualquier cosa que sepamos que no existe y de esta forma obtendremos un fichero php con un error, el cual nos revelará el path de la aplicación. Si cambiamos el valor de la variable “dir” a “../../../../../../etc/” y “file” a “passwd”, es muy probable que nos deje descargar el fichero /etc/passwd, lo mismo con cualquier otro fichero. De esta misma forma, podemos ir descargando uno a uno los códigos fuentes del sistema php y poder acceder a los usuarios y claves de conexion a la base de datos y otro tipo de información confidencial.

Me voy a basar en casos reales y tomando como ejemplo los típicos errores e irresponsabilidades de programación de los desarrolladores.

  1. < ?
  2. $nombre = $_GET[‘archivo’]; // Nombre del archivo
  3. $contenido = ‘Texto del archivo’; // Contenido del archivo
  4. $enlace = $_GET[‘folder’]."/".$_GET[‘archivo’];
  5. header ("Content-Disposition: attachment; filename=".$nombre."");
  6. header ("Content-Type: application/octet-stream");
  7. header ("Content-Length: ".filesize($enlace));
  8. readfile($enlace);
  9. ?>

Este es un script que me descargué hace un tiempo y no recuerdo de que sitio, pero es un ejemplo real y lo más probable es que aún esté en línea. Si nos fijamos línea por línea, el script realiza lo siguiente:

Lee las variables directamente desde la URL (GET) y en primera instancia, obtiene el valor de la variable “nombre” y se la asigna a $nombre, esto es para darle el nombre al archivo una vez que lo descarguemos. Luego, en la variable $enlace, guardará el path completo de lo que deseamos descargar, si le pasamos por URL los parametros folder=archivos/&archivo=test.pdf, el valor de la variable $enlace será “archivos/test.pdf”. Luego se enviarle las cabeceras al navegador, el script php leerá el fichero correspondiente al valor de la variable $enlace y forzará al navegador a descargarlo. De esta forma, si modificamos la url a nuestro modo, podremos descargar el fichero que queramos, ya que el script no tiene ninguna validación.

La forma correcta para hacer un script de este tipo, sería sin trabajar directamente con los parametros pasados por url y tenerlos encapsulados en algun lugar, ya sea en una base de datos, un switch o como sea, pero nunca directamente o bien, teniendo definido de forma explícita el directorio donde se trabaja.
Si tomamos el mismo ejemplo anterior y lo corregimos, debería quedar algo así:

  1. < ?
  2. $folder = "archivos_para_descargar";
  3. $nombre = $_GET[‘archivo’]; // Nombre del archivo
  4. $contenido = ‘Texto del archivo’; // Contenido del archivo
  5. $enlace = $folder."/".$_GET[‘archivo’];
  6. header ("Content-Disposition: attachment; filename=".$nombre."");
  7. header ("Content-Type: application/octet-stream");
  8. header ("Content-Length: ".filesize($enlace));
  9. readfile($enlace);
  10. ?>

De esta forma, jamás podremos cambiar el valor de la variable “$folder” y nunca podremos descargar un fichero que esté detrás del directorio de descarga explícitamente definido “archivos_para_descarga”. Esto, sería una forma rápida y fácil de solucionarlo, pero creo que la mejor forma es volver a hacerlo y hacerlo bien. Teniendo un “administrador” de subidas que almacene la información en una base de datos para luego realizar la descarga sólo con el ID del fichero o bien, hacer las descargas directamente con su path absoluto (https://sitio.com/archivos_para_descarga/2009/mi_documento.pdf).

Como ya he dicho, este error es muy común y basta con preparar una búsqueda con los parámetros necesarios en Google para poder acceder a sitios con estas vulnerabilidades. El problema es que con este bug, no solo comprometes la seguridad de tu sistema y de tu usuario, tambien la seguridad del resto de los usuarios del servidor, ya que el script se ejecutará como usuario www-data o apache (en la mayoría de los servidores) y tiene acceso no solo a tus ficheros.

15 comentarios

  1. Apuesto a que en la segunda version puedes pasarle igualmente ../../../passwd por el parametro $_GET[‘archivo’] sin problemas.

  2. Zerial

    octubre 18, 2009 a las 4:18 pm

    @Josep: Sep, estas en lo correcto. Para evitar eso podrías agregar una validación de que no existan “../” en la variable, pero ya tendrias un código lleno de parches y eso es asqueroso! Mejor hacerlo bien, mediante una query sql (pasando un ID) o derechamente usando el link duro hacia el fichero que deseas descargar.

  3. Wena, pero con ID igual pueden probar. Si quisiéramos que no hagan una descargar directa podría ser colocando el archivo fuera de la carpeta web y que se sólo accesible desde el archivo para descargar?
    Saben si con sesiones php se puede proteger un directorio?
    Eso, saludos.

  4. Zerial

    octubre 20, 2009 a las 1:33 pm

    @Jac: Con el ID pueden probar e ir cambiando el ID para descargar otro archivo, si, pero a lo que voy yo, es que con ese “id”, aunque lo cambies, no podras descargar un archivo arbitrariamente, menos que este en otro directorio como /etc.

    Sobre lo que preguntas.. proteger un directorio de qe forma? Puedes hacerlo por el htaccess.

    saludos

  5. Francamente no creo que el segundo script resuelva el problema, porque seguirá arrojando un error cuando no exista el fichero.
    La comprobación sobre los .., las barras etc. se deberían seguir haciendo aún cuando se use una base de datos, porque ninguna entrada del “usuario” debería ser confiable. Ni siquiera la base de datos. Por ejemplo, podría haber sido comprometida y estar siendo usada para obtener información sensible…
    Lo que yo haría es una función que elimine los caracteres no permitidos (en general lo que no es alfanumérico), junto con otra función para la ruta (no es una mala idea usar usar el DOCUMENT_ROOT como un inicio, para no llegar a comprometer a los otros sitios, aún cuando guardáramos en una base de datos el nombre del directorio de descarga) y para verificar la existencia del archivo en primer lugar. Si se necesita seguridad adicional, se podría comprobar el uid/gid del fichero en cuestión.

  6. Otra cosa que no me queda clara es la función de la variable $contenido.

  7. Zerial

    octubre 27, 2009 a las 9:57 am

    @corridear: Hola! La variable $contenido tampoco se que funcion cumple 😛 (yo copie y pegue un download.php que me descargue de un sitio).

    Sobre lo que dices en tu comentario anterior, pues si, tienes razon, pero yo solo busco “encaminar” para que encuentren el mejor metodo de validacion segun sus requerimientos.
    Es cierto lo que dices de que no se puede confiar en los parametros de entrada del usuario (como el ID), pero lo que busco yo es que no puedan hacer directory transversal, lfi o rfi

    saludos!

  8. Buscando proteger mis archivos de sql inyec. encontré esto, se bastante útil, q opinan:

    Then, when I am using my code, I simply use:

  9. jajaja no me mustra el código, ahora sí:

    function safe($value){
    return mysql_real_escape_string($value);
    }

    Then, when I am using my code, I simply use:

    $name = safe($_POST[“name”]);
    $password = safe($_POST[“password”]);

  10. Sobre proteger un directorio.

    Pero sin .htaccess, sino con php, por ej cuando validas si la sesión no está activa en un php. Validar que la sesión no vea contenido de un directorio si no está activa.
    Nosé si se puede con php.

  11. Y digo yo ¿porque nadie usa @readfile y @filesize ? poniendo la arroba antes de la función se eliminan los mensajes de error y de esta forma no apareceria el Full Path Disclosure

    También se me ocurre que podriamos tener en la carpeta (o carpetas) desde la que permitimos la descarga un fichero del tipo “fichero.txt_descarga” (un fichero vacio por ejemplo) para indicar si se puede descargar o no, por ejemplo, si queremos descargar
    $folder = “archivos/”;
    $archivo = “docu.pdf”;
    $enlace = “archivos/docu.pdf”;
    $verificacion =”archivos/docu.pdf_descarga”
    if (file_exists($verificacion)&&file_exists($enlace))
    { @readfile ($enlace); }

    Es una dia, tal vez algo cutrecilla pero pienso que efectiva ya que si intentan descargar el /etc/passwd jamas existira el fichero /etc/passwd_descargar (si existe es que nos han vulnerado el servidor por otro lado ;D )

  12. @fossie: Hola!

    Yo creo que la razon de no usar el “@” es bastante simple, pues el desarrollo de aplicaciones requieren “debug” para detectar problemas y errores, existe un “bit” para activar y desactivar los errores, se puede hacer por el php.ini, por .htaccess o bien desde el mismo codigo php, si queremos quitar los errores simplemente se desactiva la muestra de errores. Si usamos las “@” para ocultar errores, cuando queramos activar y desactivar los errores tendriamos que editar todo el codigo fuente para agregar o quitar las @ de cada metodo o funcion que utilicemos.

    saludos

  13. Encontré este código que modifiqué levemente (http://forum.codecall.net/classes-code-snippets/10316-php-directory-traversal-prevention.html) que puede ser útil para prevenir el directory transversal… solamente pasa el código si el directorio desde donde se va a bajar el archivo se encuentra más abajo (en profundidad)

    $path_parts = pathinfo($archivo);
    $directory = $path_parts[“dirname”];

    $root = explode ( DIRECTORY_SEPARATOR, realpath ( dirname ( __FILE__ ) ) );

    if (! is_dir ( $directory )) {
    die ( “Ubicación invalida.” );
    }

    $request = explode ( DIRECTORY_SEPARATOR, realpath ( $directory ) );

    empty ( $request [0] ) ? array_shift ( $request ) : $request;
    empty ( $root [0] ) ? array_shift ( $root ) : $root;

    if (count ( array_diff_assoc ( $root, $request ) ) > 0) {
    die ( “Ubicación invalida.” );
    }

    Lo otro que me resultó útil fue modificar el case de este código (http://us2.php.net/manual/en/function.header.php#102175) de manera que el default tirara un die ( “Archivo invalido.” ); para evitar extensiones no deseadas..

    Saludos

  14. Zerial, me parece muy irresponsable que coloques una solución para una vulnerabilidad y afirmes que mitiga un problema cuando no es así.

  15. Una pregunta: si conozco el directorio donde permito las descargas, podría hacer algo así? o también es inseguro:

    $nombre=$_GET[‘archivo’];
    $resultado=strpos($nombre,$directorio_de_descarga);
    if ($resultado=1){
    header (“Content-Type: application/octet-stream”);
    header (“Content-Length: “.filesize($nombre));
    readfile($nombre);
    }

Deja un comentario

Tu dirección de correo electrónico no será publicada. Los campos obligatorios están marcados con *

Esto sitio usa Akismet para reducir el spam. Aprende cómo se procesan los datos de tus comentarios.