Ho letto vari post su come consentire agli utenti di caricare file può creare vulnerabilità al tuo sito web, come un utente che inserisce il codice php in un'immagine.
Quindi ho creato un piccolo progetto di test in cui è possibile caricare (al di fuori della web root) e vedere le immagini caricate, mantenendolo così semplice come potrei avere in mente la sicurezza ma non sono un esperto e sarebbe davvero utile se potessi rispondere ad alcune delle mie domande e dirmi se qualcosa potrebbe essere fatto meglio.
Bellow è il mio progetto di prova (funzionante)
1)Do you spot anything wrong regarding permissions?
Struttura
/
public_html (root) 755
.htaccess 444
index.php 644
images.php 644
javascript 755
show_images.js 644
upload.js 644
php_scripts 755
fetch_images.php 600
upload_images.php 600
logo.png 644
secure_images
.htaccess 444
201811051007191220027687.jpg 644
20181105100719574368017.jpeg 644
secure_php_scripts 500
fetch_images.php 600
upload_images.php 600
public_html / .htaccess
#Deny access to .htaccess files
<Files .htaccess>
order allow,deny
deny from all
</Files>
#Enable the DirectoryIndex Protection, preventing directory index listings and defaulting
Options -Indexes
DirectoryIndex index.html index.php /index.php
#Trackback Spam protection
RewriteCond %{REQUEST_METHOD} =POST
RewriteCond %{HTTP_USER_AGENT} ^.*(opera|mozilla|firefox|msie|safari).*$ [NC]
RewriteCond %{THE_REQUEST} ^[A-Z]{3,9}\ /.+/trackback/?\ HTTP/ [NC]
RewriteRule .? - [F,NS,L]
Carica file correlati alle immagini
public_html / index.php
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Image upload security test</title>
<meta name="description" content="Image upload security test">
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script></head><body><formid="upload_form" method="post" enctype="multipart/form-data">
Select image to upload:
<input type="file" name="filesToUpload[]" id="filesToUpload" multiple>
<input type="submit" value="Upload Image" name="submit">
</form>
<a href="images.php">See images</a>
<script src="js/upload.js"></script>
</body>
</html>
public_html / javascript / upload.js
$("#upload_form").submit(function(e)
{
e.preventDefault();
$.ajax({
url:"../php_scripts/upload_images.php",
method:"POST",
data:new FormData(this),
processData: false,
contentType: false,
dataType:"JSON",
success:function(data)
{
if(data.outcome) { console.log("Images succesfully uploaded"); }
else { console.log(data.msg); }
}
});
});
public_html / php_scripts / upload_images.php
<?php
include_once($_SERVER['DOCUMENT_ROOT'] . "/../secure_php_scripts/upload_images.php");
2)Are the checks that i do in upload_images.php to check that the files that are being uploaded are images of the allowed format sufficient? Could i do something better?
secure_php_scripts / upload_images.php
<?php
$uploaded_images[0]["path"] = null;
try
{
$isValid = validateArray($_FILES['filesToUpload']);
if($isValid[0])
{
$data['outcome'] = true;
$data['msg'] = "Images uploaded successfully";
for($i=0; $i<count($_FILES['filesToUpload']['tmp_name']); $i++)
{
$new_name = date('YmdHis',time()).mt_rand() . "." . pathinfo($_FILES['filesToUpload']['name'][$i], PATHINFO_EXTENSION);
$path_to_be_uploaded_to = $_SERVER['DOCUMENT_ROOT'] . "/../secure_images/" . $new_name;
if(!chmod($_FILES['filesToUpload']['tmp_name'][$i], 0644) ||
!move_uploaded_file($_FILES['filesToUpload']['tmp_name'][$i], $path_to_be_uploaded_to)
)
{
$data['outcome'] = false;
$data['msg'] = "There was an error uploading your file";
break;
}
else
{
$uploaded_images[$i]["path"] = $path_to_be_uploaded_to;
}
}
}
else
{
$data['outcome'] = false;
$data['msg'] = $isValid[1];
}
echo json_encode($data);
}
catch (Exception $e)
{
//If there is an exception delete all uploaded images
if($uploaded_images[0]["path"] != null)
{
foreach($uploaded_images as $item)
{
if( file_exists($item["path"]) ) { unlink($item["path"]); }
}
}
// Also delete all uploaded files from tmp folder (Files user uploads first go there)
foreach($_FILES['filesToUpload']['tmp_name'] as $item)
{
if( file_exists($item) ) { unlink($item); }
}
$data['outcome'] = false;
$data['msg'] = "There was an error please try again later";
echo json_encode($data);
}
// Create a new blank image using imagecreatetruecolor()
// Copy our image to the new image using imagecopyresampled()
// And also add a logo in the process
function add_watermark($path_to_img, $ext)
{
try {
if($ext == 'png') { $img = imagecreatefromjpeg($path_to_img); }
else { $img = imagecreatefromjpeg($path_to_img); }
$stamp = imagecreatefrompng('logo.png');
// Set the margins for the stamp and get the height/width of the stamp image
$marge_right = 10;
$marge_bottom = 10;
$sx = imagesx($stamp);
$sy = imagesy($stamp);
list($width, $height) = getimagesize($path_to_img);
$dest_imagex = 900;//width of new image
$dest_imagey = 900;//height of new image
$dest_image = imagecreatetruecolor($dest_imagex, $dest_imagey);//create new image
imagecopyresampled($dest_image, $img, 0, 0, 0, 0, $dest_imagex, $dest_imagey, $width,$height);//#im to $dest_image
//Now $dest_image is an image of 800x800
// Copy the stamp image onto our photo using the margin offsets and the photo width to calculate positioning of the stamp.
imagecopy($dest_image, $stamp, $dest_imagex - $sx - $marge_right, $dest_imagey - $sy - $marge_bottom, 0, 0, $sx, $sy);
$filename = $path_to_img;
if($ext == 'png') { if(!imagepng($dest_image, $filename)) { return false;} }
else { if(!imagejpeg($dest_image, $filename)) { return false;} }
return true;
} catch (Exception $e)
{
return false;
}
}
// Checks
// if the element provided is a 2D array with the expected elements (multiple pictures per upload)
// if there is an error
// if file extentions are the allowed ones
// is each file size is bellow 1GB
// if the file number is less than 16
// if file exists and if it was uploaded via HTTP POST
function validateArray($array)
{
try{
if( $array && is_array($array) )
{
if( !is_array($array['name'])) { return [false, "Wrong array format"]; }
else { $pic_number = count($array['name']); }
if($pic_number > 15) { return [false, "Maximum image number allowed is 15"]; }
if( !is_array($array['type']) || count($array['type']) != $pic_number ||
!is_array($array['tmp_name']) || count($array['tmp_name']) != $pic_number ||
!is_array($array['error']) || count($array['error']) != $pic_number ||
!is_array($array['size']) || count($array['size']) != $pic_number
) { return [false, "Wrong array format"]; }
$allowedExts = array('png', 'jpeg', 'jpg');
$allowedExts2 = array('image/png', 'image/jpg', 'image/jpeg');
$fileinfo = finfo_open(FILEINFO_MIME_TYPE);
for($i=0; $i<count($array['name']); $i++)
{
if( is_array($array['name'][$i]) || is_array($array['tmp_name'][$i]) ||
is_array($array['error'][$i]) || is_array($array['size'][$i])
) { return [false, "Wrong array format"]; }
$ext = pathinfo($array['name'][$i], PATHINFO_EXTENSION);
if( !in_array($ext, $allowedExts) ) { return [false, "Only PNG JPEG JPG images are allowed"]; }
if(!file_exists($array['tmp_name'][$i]) || !is_uploaded_file($array['tmp_name'][$i])) { return [false, "File doesn't exists, try again"]; }
if(!is_uploaded_file($array['tmp_name'][$i])) { return [false, "File has to be uploaded using our form"]; }
if(!exif_imagetype($array['tmp_name'][$i])) { return [false, "Only images allowed"]; }
if(filesize($array['tmp_name'][$i]) < 12) { return [false, "All images has to be more than 11 bytes"]; }
if (!in_array(finfo_file($fileinfo, $array['tmp_name'][$i]), $allowedExts2)) { return [false, "Only PNG JPEG JPG images are allowed"]; }
if($array['error'][$i] !== 0) { return [false, "File error"]; }
if($array['size'][$i] > 1000000) { return [false, "Maximum image size allowed is 1GB"]; }
if(!add_watermark($array['tmp_name'][$i], $ext)) { return [false, "There was an error uploading your file"]; }
}
}
else { return [false, "Element provided is not a valid array"];}
return [true, "Chill dude images are ok"];
} catch (Exception $e)
{
return [false, "There was an error please try again later"];
}
}
Mostra file correlati alle immagini
public_html / images.php
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Image upload security test</title>
<meta name="description" content="Image upload security test">
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script><style>section{display:block;text-align:center;}content{display:inline-block;margin:10px;height:400px;width:400px;}contentimg{max-height:100%;max-width:100%;min-height:100%;min-width:100%;}</style></head><body><section></section><scriptsrc="js/show_images.js"></script>
</body>
</html>
public_html / javascript / show_images.js
window.onload = function() {
$.ajax({
url:"../php_scripts/fetch_images.php",
method:"POST",
dataType:"JSON",
success:function(data)
{
if(data.outcome) {
if(data.images)
{
if(data.images.length > 0)
{
let text = [];
for( let i=0; i< data.images.length; i++)
{
text[i] = "<content><img src='data:"+ data.extention[i] +";base64," + data.images[i] + "'></content>";
}
$("section").append(text);
}
}
else {console.log("no images found"); }
}
else { console.log("An error occured please try again later"); }
}
});
};
public_html / php_scripts / fetch_images.php
<?php
include_once($_SERVER['DOCUMENT_ROOT'] . "/../secure_php_scripts/fetch_images.php");
3)Fetching multiple images using
base64_encode(file_get_contents($images[$i]))
seems a bit slow and also the string that is being put inside img src is huge...can this be a problem (for example images don't appear in xiaomis MIUI browser)? Is there a better alternative?4)Let's say that a malicious image bypasses my checks during uploading. When i fetch an images using the following php code get the response in js using ajax and then append it to the dom to be shown to the user using
<img src='data:"+ data.extention[i] +";base64," + data.images[i] + "'>
is it possible to be harmful in any way?
secure_php_scripts / fetch_images.php
<?php
try
{
$data["outcome"] = true;
$directory = $_SERVER['DOCUMENT_ROOT'] . "/../secure_images/";
$images = glob($directory . "*.{[jJ][pP][gG],[pP][nN][gG],[jJ][pP][eE][gG]}", GLOB_BRACE);
$fileinfo = finfo_open(FILEINFO_MIME_TYPE);
for ($i = 0; $i < count($images); $i++)
{
$extention = finfo_file($fileinfo, $images[$i]);
header('Content-Type: ' . $extention);
$data["extention"][$i] = $extention;
$data["images"][$i] = base64_encode(file_get_contents($images[$i]));
}
echo json_encode($data);
} catch(Exception $e)
{
$data["outcome"] = false;
$data["images"][0] = [];
echo json_encode($data);
}
5)Is storing images outside of root trying to prevent access of malicious users too much of a hassle? Is it better maybe (security-speed-browser compatibility wise) to just store them inside root and make use of .htaccess to prevent someone from doing harm? Would an .htaccess like the following be sufficient for that purpose?
secure_images / .htaccess
#Deny access to .htaccess files
<Files .htaccess>
order allow,deny
deny from all
</Files>
#Enable the DirectoryIndex Protection, preventing directory index listings and defaulting
Options -Indexes
DirectoryIndex index.html index.php /index.php
#Securing directories: Remove the ability to execute scripts
AddHandler cgi-script .php .pl .py .jsp .asp .htm .shtml .sh .cgi
Options -ExecCGI