Script de carga segura

Aug 17 2020

Estoy creando una red social que permite a los usuarios subir una foto de perfil. Solo quiero saber si esta es una forma segura de hacerlo. Gracias.

<?php

include 'includes/header.php';
include 'includes/form_handlers/settings_handler.php';

//$userPic = ''; $date_time = date('Y-m-d_H-i-s');

if(!empty($userLoggedIn)) { if (isset($_FILES['fileToUpload'])) {
        $errors= array(); $file_name = $_FILES['fileToUpload']['name']; $file_size = $_FILES['fileToUpload']['size']; $width = 1500;
        $height = 1500; $file_tmp = $_FILES['fileToUpload']['tmp_name']; $file_type = $_FILES['fileToUpload']['type']; $tmp = explode('.',$_FILES['fileToUpload']['name']); $file_ext=strtolower (end ($tmp)); $extensions = array( "jpeg", "jpg", "png", "gif");

        if(in_array($file_ext,$extensions)=== false){

            $errors[]="extension not allowed, please choose a JPEG or PNG file."; } if ($file_size > 8097152) {

            $errors[] = 'File size must be 2 MB'; } if ($width > 1500 || $height > 1500) { echo"File is to large"; } if(!$errors) {

            $userPic = md5($_FILES["fileToUpload"]["name"]) . $date_time . " " . $file_name;
            $profilePic = move_uploaded_file($file_tmp,"assets/images/profile_pics/" . $userPic); $file_path = "assets/images/profile_pics/" . $userPic; $stmt = $con->prepare("UPDATE users SET profile_pic = ? WHERE username = ?"); $stmt->bind_param('ss', $file_path, $username);
            $stmt->execute(); $stmt->close();

            header('Location: settings.php');
            exit();
 
        }
    }
} else {

    echo "Invalid Username";
}

?>

Respuestas

5 LiamSorsby Aug 18 2020 at 21:13

Esta es mi opinión personal, pero diría lo siguiente:

  1. El código debe estar formateado, personalmente miraría al PSR-12 ya que este estándar debe seguirse cuando sea posible.
  2. move_uploaded_file no protege contra el cruce de directorios. Debe usar el nombre de base en $_FILES['fileToUpload']['tmp_name']y algunas otras formas de validación
  3. Verificar la extensión del archivo con if(in_array($file_ext,$extensions)=== false)no evita que un usuario cargue un archivo malicioso; por ejemplo, podría usar un byte mágico para engañar al servidor y hacerle creer que es un determinado tipo de archivo. Debería echar un vistazo a finfo y al primer ejemplo de carga de archivos
  4. Está creando una serie de errores, que actualmente se verifica en una declaración if y luego se desecha. Si no está planeando usarlo, es mejor que regrese temprano de la función en lugar de continuar con la ejecución.
  5. Dependiendo de lo único que deba ser el nombre de archivo, es posible que desee usar algo como uniqid(mt_rand(), true)
  6. move_uploaded_file reemplazará un archivo si ya existe, es posible que desee comprobar que existe antes de sobrescribir un archivo existente. Dependiendo de su solución de nomenclatura, es muy poco probable que ocurra, pero bajo una carga alta durante largos períodos de tiempo, esto podría suceder con más frecuencia de lo que cree.
  7. Está usando UPDATE users SET profile_pic = ? WHERE username = ?, supongo que este valor existe en la base de datos, ya que el usuario debe iniciar sesión. Sin embargo, si no está seguro de si el campo existe o no (no he visto la base de datos) uso personal: INSERT INTO users (profile_pic, username) VALUES (?,?) ON DUPLICATE KEY UPDATE profile_pic=?, username=?esto se insertará en la tabla si la fila no existe, pero la actualizará si existe.
  8. Ha establecido una variable local llamada ancho y alto y los está comparando con el mismo valor. ¿Supongo que esto estaba destinado a verificar las dimensiones reales del archivo?

Espero que esto ayude de alguna manera :)