PHP Descargar contador de script con htaccess -- php campo con security camp codereview Relacionados El problema

PHP download counter script with htaccess


5
vote

problema

Español

Me gustaría mostrarle mi guión y me gustaría escuchar sus sugerencias de seguridad. Tal vez el código también sea útil para otra persona.

lo que quiero archivar:

  1. alguien hace clic en un enlace en mi página web
  2. Un archivo HTACCESS busca algunas extensiones de archivos y si el archivo solicitado es uno de estos, se contará.
  3. Después del recuento (hecho con PDO Prepare), el archivo se realiza al usuario.

Porque quiero obtener la máxima seguridad y no ofrecer a alguien otros archivos que desde la carpeta de carga que solicite antes de hacerlo mal.

.htaccess :

  RewriteEngine on RewriteRule ^(.*).(rar|zip|pdf|tar|apk|rpm|exe)$ /download.php?file=$1.$2 [L,NC]   

descargar.php

  <?php  // Allowed extensions $filetypes = array("rar", "zip", "pdf", "tar", "apk", "rpm", "exe");  // Details of requested file $file_path  = $_REQUEST['file'];  // NullByte cleaner $file_path = str_replace(chr(0), '', $file_path);  // Split details $path_parts = pathinfo($file_path); $file_name  = $path_parts['filename']; $file_ext   = $path_parts['extension'];  // CleanUp $file_name  = preg_replace('/[^-9_a-zA-ZäöüÄÖÜ]+/', '', $file_name); $file_ext  = preg_replace('/[^-9a-zA-Z]+/', '', $file_ext);  // Create path $dir_path = $_SERVER['DOCUMENT_ROOT']."/"; $finalpath  = $dir_path . $file_name . "." . $file_ext;  // Allowed extension? if ( in_array($file_ext, $filetypes) ) {      // Does the file exist?     if ( is_file($finalpath) ) {          // Final security check         setlocale(LC_ALL,'en_US.UTF-8');         $local_ext = pathinfo($finalpath, PATHINFO_EXTENSION);          // Allowed local extension?         if ( in_array($local_ext, $filetypes) ) {              // Logging             // ..........              // Download             header('Content-Description: File Transfer');             header('Content-Type: application/octet-stream');             header('Content-Disposition: attachment; filename=' . basename($finalpath));             header('Content-Transfer-Encoding: binary');             header('Expires: 0');             header('Cache-Control: must-revalidate, post-check=0, pre-check=0');             header('Pragma: public');             header('Content-Length: ' . filesize($finalpath));             ob_clean();             flush();             readfile($finalpath);             exit;         }      }  }  // Something went wrong echo "Error..."; exit;   
Original en ingles

I'd like to show you my script and would like to hear your security hints. Maybe the code is also useful to someone else.

What I want to archive:

  1. Someone clicks on a link on my web page
  2. a htaccess file looks for some file-extensions and if the requested file is one of these, it will be counted.
  3. After the count (done with PDO prepare), the file is proceed to the user.

Because I want to get maximum security and not offering someone other files than from the upload folder I ask before doing it wrong.

.htaccess:

RewriteEngine on RewriteRule ^(.*).(rar|zip|pdf|tar|apk|rpm|exe)$ /download.php?file=$1.$2 [L,NC] 

download.php

<?php  // Allowed extensions $filetypes = array("rar", "zip", "pdf", "tar", "apk", "rpm", "exe");  // Details of requested file $file_path  = $_REQUEST['file'];  // NullByte cleaner $file_path = str_replace(chr(0), '', $file_path);  // Split details $path_parts = pathinfo($file_path); $file_name  = $path_parts['filename']; $file_ext   = $path_parts['extension'];  // CleanUp $file_name  = preg_replace('/[^\0-9_a-zA-Zxc3xa4xc3xb6xc3xbcxc3x84xc3x96xc3x9c]+/', '', $file_name); $file_ext  = preg_replace('/[^\0-9a-zA-Z]+/', '', $file_ext);  // Create path $dir_path = $_SERVER['DOCUMENT_ROOT']."/"; $finalpath  = $dir_path . $file_name . "." . $file_ext;  // Allowed extension? if ( in_array($file_ext, $filetypes) ) {      // Does the file exist?     if ( is_file($finalpath) ) {          // Final security check         setlocale(LC_ALL,'en_US.UTF-8');         $local_ext = pathinfo($finalpath, PATHINFO_EXTENSION);          // Allowed local extension?         if ( in_array($local_ext, $filetypes) ) {              // Logging             // ..........              // Download             header('Content-Description: File Transfer');             header('Content-Type: application/octet-stream');             header('Content-Disposition: attachment; filename=' . basename($finalpath));             header('Content-Transfer-Encoding: binary');             header('Expires: 0');             header('Cache-Control: must-revalidate, post-check=0, pre-check=0');             header('Pragma: public');             header('Content-Length: ' . filesize($finalpath));             ob_clean();             flush();             readfile($finalpath);             exit;         }      }  }  // Something went wrong echo "Error..."; exit; 
     

Lista de respuestas

2
 
vote

estás haciendo muchas cosas bien. Usted está utilizando una lista blanca en lugar de una lista negra para extensiones, está utilizando PathInfo para obtener la extensión, limpia NullBytes (opcional, pero aún buena práctica), utiliza el nombre basen en el nombre de archivo (también opcional), tiene una lista blanca estricta para Nombres de archivo (teóricamente), utiliza un buen tipo de contenido, y así sucesivamente.

Hay un problema, y ​​dos pequeñas sugerencias:

  • La lista blanca de nombre de archivo permite a todos los caracteres entre ASCII 0 y ASCII 57, por ejemplo, NUL , TAB1 , "2 , 99887776655443333 , <4 , etc. Dado el resto de su código, asumo que este es un error y que solo quería permitir al contenido alfanum + UMLAUT. Lo mismo ocurre con el filtro de extensión.
  • Nunca hay una buena razón para usar REQUEST . Probablemente usted conozca el método HTTP que está utilizando, por lo que debe usarlo. Esto es algo de problema si el método es publicado, ya que un atacante puede rebajarse para obtenerlo, lo que facilita la explotación de algunos problemas.
  • Es posible que desee considerar la comprobación de si el RESOLVED $ FINALPATH está dentro del directorio de descargas. Esto no es necesario en este momento, ya que su enfoque de PathInfo, así como su lista blanca, ambos, por su cuenta, son lo suficientemente defensores contra el Traversal de directorios. Pero es la mejor práctica, y puede ser útil si el código se reutiliza en un contexto diferente, o si su enfoque de WHITELIST (y PATHINFO) debe cambiarse por alguna razón.

Con respecto a los aspectos de la no seguridad:

  • Sus comentarios no agregan todo eso y probablemente debería ser eliminado. Algunos se pueden usar para crear mejores nombres de variables ( filetypes puede ser allowedExtensions ), otros simplemente están repitiendo el código, y algunos son algo poco claros (¿qué es una extensión local?) .
  • Tu IFS es un poco anidado demasiado para mi gusto. Si invierte el IFS, serán más fáciles de leer. Si regresa / muere en el caso, también se deshará de la anidación.
 

You are doing a lot of things right. You are using a whitelist instead of a blacklist for extensions, you are using pathinfo to get the extension, you clean nullbytes (optional, but still good practice), you use basename in the filename (also optional), you have a strict whitelist for filenames (theoretically), you use a good content type, and so on.

There is one issue, and two small suggestions:

  • Your filename whitelist allows all characters between ascii 0 and ascii 57, eg NUL, TAB, !, ", <, etc. Given the rest of your code, I'm assuming that this is a bug and that you only wanted to allow alphanum + umlaute. The same is true for the extension filter.
  • There is pretty much never a good reason to use REQUEST. You probably know the HTTP method you are using, so you should use that. This is somewhat of an issue if the method is POST, as an attacker can downgrade that to GET, which can make exploitation of some issues easier.
  • You may want to consider checking if the resolved $finalpath is inside the download directory. This isn't necessary right now, as your pathinfo approach as well as your whitelist both on their own are defense enough against directory traversal. But it is best-practice, and it may be useful if the code is reused in a different context, or if your whitelist (and pathinfo) approach need to be changed for some reason.

Regarding non-security aspects:

  • Your comments do not add all that much and should probably be removed. Some can be used to create better variable names (filetypes might be allowedExtensions), others are just repeating the code, and some are somewhat unclear (what is a local extension?).
  • Your ifs are a bit too nested for my taste. If you reverse the ifs they will be easier to read. If you return/die on the else case, you would also get rid of the nesting.
 
 
     
     

Relacionados problema

6  Programa de recuperación de contraseña  ( Password recovery program ) 
Este es un programa de recuperación de contraseña que hice, y solo quiero que se revise. Estos no son todos los archivos para el sistema de inicio de sesión y...

1  Simple Node.js WebServer  ( Simple node js webserver ) 
Estoy en el proceso de escribir un servidor web simple.js. Estoy a mitad de camino en términos de funcionalidad. Consulte el código completo en este pastebin...

11  ¿Es esta una forma suficiente de prevenir las inyecciones de guiones y otras cosas malas en las cuerdas?  ( Is this a sufficient way to prevent script injections and other bad stuff in str ) 
¿Esta función será suficiente para eliminar todo el código malicioso y los caracteres extraños de una cadena? import java.security.SecureRandom; import jav...

1  ¿Cómo puedo determinar y probar Vunerabilidades de inyección de SQL en este código de inicio de sesión?  ( How can i ascertain and test sql injection vunerabilities on this login code ) 
Sé que el escenario "Little Bobby Tablas" y se estaba preguntando si este código es vulnerable a tales inyecciones de SQL. Soy bastante nuevo en PHP y teng...

2  Formulario básico de comentarios de PHP  ( Basic php comment form ) 
Soy un novato de programación. He escrito este simple script PHP para ejecutar un formulario de comentarios muy básico y apreciaría cualquier comentario, espe...

51  Cifrado seguro simplificado de una cadena  ( Simplified secure encryption of a string ) 
Tengo dos ejemplos de código que escribí para las mejores prácticas cifrando una cadena que busco comentarios tanto en términos de mejores prácticas y segurid...

11  Cifrado simétrico en C #  ( Symmetrical encryption in c ) 
Estoy tratando de cifrar simétricamente algunos datos utilizando C #, y parece que hay mucha información engañosa o incorrecta en el sujeto. He creado un pr...

3  Formulario de contacto de WordPress PHP - Fallas de seguridad  ( Wordpress php contact form security flaws ) 
Tengo el siguiente formulario de contacto incluido con mi tema de WordPress como plantilla. Ahora, un usuario en StackOverFlow señaló que este formulario ti...

10  Generando tokens de sesión difícil de adivinar  ( Generating hard to guess session tokens ) 
¿Este código crea tokens de sesión suficientemente difíciles de adivinar, asumiendo que el servidor y el cliente se comunican a través de HTTPS? Tomar 2 ...

8  Convertir un segurreque en una matriz de bytes  ( Converting a securestring to a byte array ) 
¿Asigna algo que no ha liberado? ¿Algo que permanece en la memoria? Excepto la matriz de bytes de resultado, OFC. checkOnline.sh1 ...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos