images upload safely !?

ajoo · January 1, 2017

Hi all,

It's business as usual at PHP freaks. I'ld still like to wish a very Happy New Year to all the great Gurus, Admins and members of the forum !!

I have put together a script for uploading images. I know this happens to be a very dicey area of php development and hundreds of similar, yet different scripts on the net make it all the more so since it's impossible to find a right solution, if at all one exists.

So here is my small script ( working ), which previews a resized image, save it to a db and finally retrieves it from the DB to display it again. I would like to know if this is safe. If not what else needs to be done to make it as secure as possible.

usp6_phpfreaks.php

<?php
error_reporting(E_ALL);
require_once 'db.php'; 
define("HALF_MB", 512000);
define("SV_WIDTH", 150);
define("SV_HEIGHT", 175); 

$uploaddir = 'D:/xampp/php/internals/uploads/'; # Outside of web root

if(isset($_POST['upload']) && $_POST['upload']==="Upload"){

	$blob = "BLOB";
	$filename = $_FILES['userfile']['name'];
	$filetype = $_FILES['userfile']['type'];
	$tmp_filename = $_FILES['userfile']['tmp_name'];
	$size = filesize($tmp_filename);
	
	if($filename)
	{
		$filename = stripslashes($filename);
		$uploadfile = tempnam($uploaddir, "upm"); 	
	//	echo $uploadfile;		// Outputs :  D:\xampp\php\internals\uploads\upm12D7.tmp
	//	exit();
		
	}else $error = 1;
	
	if($size>HALF_MB) $error = 2;
		
	$extension = strtolower(getExtension($filename));
//	echo "Ext : ".$extension;
//	exit();

	
	if 	(($extension === "jpg") || ($extension === "jpeg") || ($extension === "png") || ($extension === "gif"))
	{

		if($extension==="jpg" || $extension==="jpeg" )
		{
			$src = imagecreatefromjpeg($tmp_filename);
		}
		elseif($extension==="png")
		{
			$src = imagecreatefrompng($tmp_filename);
		}
		else 
		{
			$src = imagecreatefromgif($tmp_filename);
		}

	}else $errors=3;

	list($ix, $iy, $type, $attr) = getimagesize($tmp_filename);

	$widthscale = $ix/SV_WIDTH;  //<TD> WIDTH
	$heightscale = $iy/SV_HEIGHT; //<TD> HEIGHT

	if($widthscale < 1) $nwidth = $ix*$widthscale;
	else $nwidth = $ix/$widthscale;

	if($heightscale < 1) $nheight = $iy*$heightscale;
	else $nheight = $iy/$heightscale;

	$dst = imageCreateTrueColor($nwidth, $nheight);
	imageCopyResampled($dst, $src, 0, 0, 0, 0, $nwidth, $nheight, $ix, $iy);

	$imagemoved = imagepng($dst,$uploadfile);
	imagedestroy($src);
	imagedestroy($dst);
	
		
		if ($imagemoved) 
		{
		
			try
			{
				$query = "INSERT INTO images (name, original_name, mime_type, images) VALUES (?,?,?,?)";

				$stmt = $link->prepare($query);
				$stmt->bind_param('ssss',$uploadfile,$filename,$filetype,$blob);
				if($stmt->execute()){
					
					$id=$stmt->insert_id;	// get the just inserted id
					echo "ID = ".$id;
				}
				else{
					
					echo "Error : ".$link->error;
				}
				
			}
			catch(mysqli_sql_exception $registrationException)
			{
				if ($registrationException->getCode() === MYSQL_ER_DUP_ENTRY)
				{
					echo'The username is already taken, please choose a different name.';
					return false;
				}
				else
				{
					// rethrow exception
					throw $registrationException;
				}
			}	
		}	
	echo "File is valid, and was successfully uploaded. You can view it <a href='ups6_view.php?id=$id'>here</a>";
}


function getExtension($str) {

	$i = strrpos($str,".");
	if (!$i) { return ""; } 
	$l = strlen($str) - $i;
	$ext = substr($str,$i+1,$l);
	return $ext;
 }
 
 
?>

<html>
	<head>

		<title>File Upload To Database</title>

		<link class="jsbin"  href="https://ajax.googleapis.com/ajax/libs/jqueryui/1.12.1/themes/smoothness/jquery-ui.css" rel="stylesheet" type="text/css" >
		<link rel="stylesheet" type="text/css" media="screen" href="divs.css"  />
		
		<script class="jsbin" src="http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js"></script>
		<script class="jsbin" src="http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.0/jquery-ui.min.js"></script>
		<meta charset=utf-8 />
	
	</head>

<body>
	
		<form name="upload" action="#" method="POST" enctype="multipart/form-data">

			Select the file to upload: 	<br>
			
			<input type="file" name="userfile" onchange="readURL(this)"; /><br>
			<br><input type="submit" name="upload" value="Upload">
			<div class = "div1">
			<img id="image" src="#" alt="load image" />		<!-- This displays the image. -->
			</div>
		
		</form>

	<script>
		function readURL(input) {
			if (input.files && input.files[0]) 
			{
				var reader = new FileReader();
				reader.onload = function (e) {
				  $('#image')
					.attr('src', e.target.result)
					.width(160)
					.height(185);
				};
				reader.readAsDataURL(input.files[0]);
			}else{
				alert( "File not selected or browser incompatible." );
			}
		}
	</script>
</body>
</html>

and

usp6_view.php

<?php
error_reporting(E_ALL);
require_once 'db.php';

$uploaddir = 'D:/xampp/path_to_any_folder/uploads/';  // set your path here.
$id = $_GET['id'];

	if(!is_numeric($id)) 
	{
		die("File id must be numeric");
	}

	$query = 'SELECT name, mime_type FROM images WHERE id=?';

	$stmt = $link->prepare($query);
	$stmt->bind_param('i',$id);
	$stmt->execute();	
	$stmt->bind_result($filename,$type);
	$stmt->store_result();
	$stmt->fetch();	
	
	$stmt->free_result();

	if(is_null($filename) || count($filename)===0)
	{
		die("File not found");
	}
	

	header("Content-Type: " . $type);
	echo '<img width="150" height="150" src= "'.readfile($filename).'"alt="">'; 

?>
<head>

	<title>Divs</title>
		<link rel="stylesheet" type="text/css" media="screen" href="divs.css"  />
	<meta charset=utf-8 />
	
</head>

Of course this requires a small DB for storing and retrieving images which should be simple enough to create if someone wishes to try it out.

While reading on the subject I also came across a relatively new attack which uses images to attack sites and is know as Stegosploit, How do we protect against attacks like these?

So thanks again and looking for some cool security tips and measures from you all,

Once again A happy new year !

Thanks all.

ginerjm · January 1, 2017

One thing I would point out. The practice of saving images in a db table is not the most efficient way to save them. Since images are files unto themselves there is no reason for storing them in a db. They should be simply saved in a dedicated folder or folder tree and referenced via a constant for the application or as a pathname in the db and not be written to the db.

Jacques1 · January 1, 2017

At least the view part definitely doesn't work. You set the Content-Type header to the image type, but then you try to output HTML. You also try to use readfile() to insert the image content into an img element, but readfile() doesn't return the image, and even if it did, you can't just insert bytes into an image source (you'd have to use data URLs with an encoded payload, but this has other problems like efficiency).

Without actual working code, we cannot really assess the risks. However, there are many conceptual security problems:

The content type declaration, one of the most critical aspects, is accepted directly from the user input ($_FILES[...]['type']) without any validation at all. This means the user can, for example, upload a file with malicious JavaScript code and actually declare it as text/html or text/svg. If you deliver the file with this type, you trigger code execution.
Even when you add validation to the upload script, there's still no validation in the view script. So if a user chooses a harmless type during upload and then manages to manipulate it later (e. g. through an SQL injection), they can again make you trigger code execution.
You have a uniqueness check during insertion. If you only allow unique filenames, it will be easy to perform a denial-of-service attack by uploading files with common names.
The random filenames generated by tempnam() are fairly weak and may again be abused for a denial-of-service attack.

You also have no proper error checking. PHP errors during upload are completely ignored, and even when you do detect an error (e. g. a wrong type), you just set an error variable and let the script keep running. Deleting the original image isn't a good idea either, because resizing is a destructive operation. You cannot go back if you realize that something went wrong or you simply want a better quality.

I would start from scratch and use a much stricter and more robust approach:

Create a hard-coded whitelist of allowed extensions and MIME types. This will be used in multiple scripts, so put it into a central place (e. g. the configuration script).
When you receive an upload, you first check for errors. If the upload has failed, you abort processing.
Then you check the file extension and/or the MIME type. If this fails, you again abort processing. You must not continue, because this may be an attack.
You use a strong random number generator (like the one I gave you) to generate the filename and store the original(!) file outside of the document root.
Then you can additionally store resized images.

The easiest and most efficient way of delivering the images is through the “sendfile” mechanism (e. g. mod_xsendfile for Apache). This delegates the task to the webserver rather than PHP.

In any case, make sure that all files are served with a safe MIME type.

ajoo · January 1, 2017

Hi !

@ginerjm : Thanks for that pointer. I have read a lot about this issue but no one seemed to put one practice above another. But yes I'll keep that in mind.

@Guru Jacques:

At least the view part definitely doesn't work.

Hi Sir, do you mean that the code in usp6_view.php does not work - like does not give an output OR do you mean that it's not good because of the issues you have highlighted? In case you meant the latter, then I can say for sure ( having tested it yet again ) that it works and once the link is clicked, the image is fetched and displayed. The <head ... / head> bit in ups6_view.php can be commented out.

As for the rest of the issues you mention, I'll go through in detail, make changes and revert with the modified code here again.

Thanks you both !

Jacques1 · January 1, 2017

In case you meant the latter, then I can say for sure ( having tested it yet again ) that it works and once the link is clicked, the image is fetched and displayed.

When you open the image in a hex editor, you'll see garbage after the image data:

{DATA_OF_PNG_IMAGE}<img width="150" height="150" src= "{BYTE_LENGTH_OF_IMAGE}"alt=""><head>...

The browser may ignore this nonsense data and display the image regardless, but that doesn't change the fact that the code is incorrect and generates nonsense data.

ajoo · January 2, 2017

Hi Guru Jacques,

I'ld like to go this one by one with you and for the time I'll forget about the view - retrieving from the DB - part .

The content type declaration, one of the most critical aspects, is accepted directly from the user input ($_FILES[...]['type']) without any validation at all. This means the user can, for example, upload a file with malicious JavaScript code and actually declare it as text/html or text/svg. If you deliver the file with this type, you trigger code execution.

Rectified in the code. I overlooked this. I did check for file extension ($extension), but used the other $filetype in the query. Changed it to $extension now.

You have a uniqueness check during insertion. If you only allow unique filenames, it will be easy to perform a denial-of-service attack by uploading files with common names.

I think I can simply do away with unique filenames since the ID is unique in each case and the data is directly available in the DB. I am using the DB for these images as their size is really small. ( resizing the images for the same reason ). Even the limit of 512KB is far more than reqd. I read that for small image sizes, it's ok to have them in the DB as that results in speedier retrival. However Ginerjm also suggested that I should not use the DB. Sir, what's your opinion?

The random filenames generated by tempnam() are fairly weak and may again be abused for a denial-of-service attack.

Changed that. Using substr(hash('sha512',rand()),0,12) for the time being, for brevity to avoid the library inclusion etc. Will use random_bytes() in the real code as you have suggested.

Create a hard-coded whitelist of allowed extensions and MIME types. This will be used in multiple scripts, so put it into a central place (e. g. the configuration script).

created a function fcheckMimeType with a whitelist array.

When you receive an upload, you first check for errors. If the upload has failed, you abort processing.

Then you check the file extension and/or the MIME type. If this fails, you again abort processing. You must not continue, because this may be an attack.

implemented, hopefully correctly :happy-04:

You use a strong random number generator (like the one I gave you) to generate the filename and store the original(!) file outside of the document root.

doing so already.

Deleting the original image isn't a good idea either, because resizing is a destructive operation. You cannot go back if you realize that something went wrong or you simply want a better quality.

SIr, I read that despite the precautions taken above, messages or for that malicious code can be added to images, which will bypass all the above precautions. It also said that if images are resized, then that message / code is eliminated. I am not sure if resizing the image as in my code will actually eliminate that risk or not. Therefore I chose not to save the image. Besides, with the error checking in place, if all goes well and the image is saved in the database only then is the actual image deleted. In fact I don't even need to save the image at all since I get it's dump in the DB. Most images would be passport sized photos and so chances are remote that there would be a quality / resolution issue. There is also the preview.

Still I would very much like your views on this above. I would be happy to know how you would do it if you had to.

I have also used an online hex editor to view the files, i just created, but failed to see any garbage in the binary data. Kindly check the output once again. I think the previous bit of code was able to handle only png files. Maybe that was the reason, but i cannot be sure. Kindly check again if possible. I will however check the retrieval bit now according to the method you mentioned in your previous reply.

If the code seems fine now, I'll get on with the retrieval part. Please find attached below the modified code for ups6_phpfreaks.php

Thanks loads !

<?php
error_reporting(E_ALL);
require_once 'db.php';
define("HALF_MB", 512000);    // defines will go into the config file.
define("SV_WIDTH", 150);
define("SV_HEIGHT", 175);

$uploaddir = 'D:/xampp/php/internals/uploads/'; // Outside of web root
$rec_insert = 'none';

if(isset($_POST['upload']) && $_POST['upload']==="Upload"){

    $error_msg="";    
    $blob = "BLOB";
    
    $filename = $_FILES['userfile']['name'];
    $filetype = $_FILES['userfile']['type'];
    $tmp_filename = $_FILES['userfile']['tmp_name'];
    $file_error=$_FILES['userfile']['error'];
    
    $size = filesize($tmp_filename);
    $imageinfo = getimagesize($tmp_filename);
    $mime = $imageinfo['mime'];
    $extension = strtolower(getExtension($filename));

    if($file_error>0) $error_msg = 'Error uploading file.';
    if($size>HALF_MB) $error_msg = "File size too big";    
    if(!fcheckMimeType($mime)) $error_msg = 'Invalid file type';        
    
    if($error_msg === "")        // no errors flagged
    {
        $uploadfile = substr(hash('sha512',rand()),0,12);    // For brevity to avoid adding the lib & it's function. Will use bin2hex(random_bytes(FILE_NAME_LENGHT)); in the real code.
        $uploadfile = $uploaddir.$uploadfile.".tmp";
    //    echo $uploadfile;

        if     (($extension === "jpg") || ($extension === "jpeg") || ($extension === "png") || ($extension === "gif"))
        {

            if($extension==="jpg" || $extension==="jpeg" )
            {
                $src = imagecreatefromjpeg($tmp_filename);
            }
            elseif($extension==="png")
            {
                $src = imagecreatefrompng($tmp_filename);
            }
            else
            {
                $src = imagecreatefromgif($tmp_filename);
            }

        }else $errors=3;

        list($ix, $iy, $type, $attr) = getimagesize($tmp_filename);

        $widthscale = $ix/SV_WIDTH; //<TD> WIDTH
        $heightscale = $iy/SV_HEIGHT; //<TD> HEIGHT

        if($widthscale < 1) $nwidth = $ix*$widthscale;
        else $nwidth = $ix/$widthscale;

        if($heightscale < 1) $nheight = $iy*$heightscale;
        else $nheight = $iy/$heightscale;

        $dst = imageCreateTrueColor($nwidth, $nheight);
        imageCopyResampled($dst, $src, 0, 0, 0, 0, $nwidth, $nheight, $ix, $iy);
        
        // The last example had only png handling. Maybe the reason for the garbage in the image binary data
        if($extension === 'png') $imagemoved = imagepng($dst,$uploadfile);
        if($extension === 'jpg' || $extension === 'jpeg') $imagemoved = imagejpeg($dst,$uploadfile);
        if($extension === 'gif') $imagemoved = imagegif($dst,$uploadfile);
        imagedestroy($src);
        imagedestroy($dst);
        
        
        if ($imagemoved)
        {
        
            try
            {
                $query = "INSERT INTO images (name, original_name, mime_type, images) VALUES (?,?,?,?)";

                $stmt = $link->prepare($query);
                $stmt->bind_param('ssss',$uploadfile,$filename,$extension,$blob);
                if($stmt->execute()){
                    
                    $id=$stmt->insert_id;    // get the just inserted id
                    $rec_insert = 'inserted';
                //    echo "ID = ".$id;

                }
                else{
                    $rec_insert = 'failed';
                    echo "Error : ".$link->error;
                }
                
            }
            catch(mysqli_sql_exception $registrationException)
            {
                if ($registrationException->getCode() === MYSQL_ER_DUP_ENTRY)
                {
                    echo'The filename is already taken, please choose a different name.';
                    $rec_insert = 'failed';
                    return false;
                }
                else
                {
                    // some other exception
                    throw $registrationException;
                }
            }    
        }else echo "Dismissed";    
        
    }else echo "<br>".$error_msg."<br>";
    
    if($rec_insert === 'inserted') echo "File is valid, and was successfully uploaded. You can view it <a href='ups6_view.php?id=$id'>here</a>";
    if($rec_insert === 'failed') echo "Record could not be inserted";

}




function getExtension($str) {

    $i = strrpos($str,".");
    if (!$i) { return ""; }
    $l = strlen($str) - $i;
    $ext = substr($str,$i+1,$l);
    return $ext;
}

function fcheckMimeType($mimetype)
{
    $mimes    = array('image/gif','image/jpeg','image/png');
    $testmime = filter_var(in_array($mimetype,$mimes),FILTER_VALIDATE_BOOLEAN)? TRUE : FALSE;
    // if(!$testpage) $ppage = false;
    return $testmime;
}

?>

<html>
    <head>

        <title>File Upload To Database</title>

        <link class="jsbin" href="https://ajax.googleapis.com/ajax/libs/jqueryui/1.12.1/themes/smoothness/jquery-ui.css" rel="stylesheet" type="text/css" >
        <link rel="stylesheet" type="text/css" media="screen" href="divs.css" />
        
        <script class="jsbin" src="http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js"></script>
        <script class="jsbin" src="http://ajax.googleapis.com/ajax/libs/jqueryui/1.8.0/jquery-ui.min.js"></script>
        <meta charset=utf-8 />
    
    </head>

<body>
    
        <form name="upload" action="#" method="POST" enctype="multipart/form-data">

            Select the file to upload:     <br>
            
            <input type="file" name="userfile" onchange="readURL(this)"; /><br>
            <br><input type="submit" name="upload" value="Upload">
            <div class = "div1">
            <img id="image" src="#" alt="load image" />        <!-- This displays the image. -->
            </div>
        
        </form>

    <script>
        function readURL(input) {
            if (input.files && input.files[0])
            {
                var reader = new FileReader();
                reader.onload = function (e) {
                 $('#image')
                    .attr('src', e.target.result)
                    .width(160)
                    .height(185);
                };
                reader.readAsDataURL(input.files[0]);
            }else{
                alert( "File not selected or browser incompatible." );
            }
        }
    </script>
</body>
</html>

Jacques1 · January 2, 2017

You're using the MIME type determined by the server to do the whitelist check, but then you're using the extension to determine the image type again for your conversions. This can lead to errors, because the MIME type and extension may contradict each other. Pick one and stick to it. For example, use the MIME type for everything and just ignore the extension.

Loading the images from the database and serving them with PHP is not faster than serving static files. In fact, you have to do a lot of extra work if you want standard features like caching. The database also gets significantly bigger (which can be a problem for backups), and it's harder to access the files on the server (you can't just download them with your standard SFTP tool). If you have a good(!) reason why you want the images in the database, that's fine. But the standard approach is to use plain files.

Storing raw images on the server does not affect security, especially when those files aren't even sent to the user and just act as a backup. But if you don't think you need this, that's up to you.

ajoo · January 3, 2017

Hi

You're using the MIME type determined by the server to do the whitelist check, but then you're using the extension to determine the image type again for your conversions. This can lead to errors, because the MIME type and extension may contradict each other. Pick one and stick to it. For example, use the MIME type for everything and just ignore the extension.

:thumb-up:

Loading the images from the database and serving them with PHP is not faster than serving static files. In fact, you have to do a lot of extra work if you want standard features like caching. The database also gets significantly bigger (which can be a problem for backups), and it's harder to access the files on the server (you can't just download them with your standard SFTP tool). If you have a good(!) reason why you want the images in the database, that's fine. But the standard approach is to use plain files.

It's obvious that both Ginerjm and you are both suggesting that I take the approach of saving the files rather than the it's binary blob in the DB. So I guess I will change to that approach since I don't really have a good reason for doing it the otherway. I thought it was simple for smaller files and faster ( like I said I read but which you said is not really so). So all I need to do there is save the image name instead of the blob in the DB.

SIr, I read that despite the precautions taken above, messages or for that malicious code can be added to images, which will bypass all the above precautions. It also said that if images are resized, then that message / code is eliminated. I am not sure if resizing the image as in my code will actually eliminate that risk or not.

Any comments about this ?

I have also used an online hex editor to view the files, i just created, but failed to see any garbage in the binary data. Kindly check the output once again.

Even though I am going to do away with the binary data, I would still like to know if this garbage is still showing or was it because of the code handling just one type of image. If possible, kindly check it once again since I found did not find any garbage appended to the end of the binary image data.

Even when you add validation to the upload script, there's still no validation in the view script. So if a user chooses a harmless type during upload and then manages to manipulate it later (e. g. through an SQL injection), they can again make you trigger code execution.

If I was to use the image blob for displaying the image in view, would I need to validate the binary data? what sort of validation would be needed here ???

Thanks loads.

Jacques1 · January 3, 2017

Any comments about this ?

The fact that code can be hidden in images is well-known. However, it still needs to be triggered, and as far as I can tell, “Stegosploit” doesn't offer any new techniques in that area. It's just a cleverer way of hiding the code.

In general, I would concentrate on preventing code execution rather than trying to remove the code through image manipulations. The most robust way is to make sure the files are delivered with an explicit and safe Content-Type header. Modern browsers respect this declaration and do not execute the embedded code. Some older browsers (especially Internet Explorer) are less reliable, but the users still running around with IE 6/7/8 have bigger problems.

Sure, you can try to use resizing as an extra defense mechanism. But the MIME type is still the primary security factor.

Even though I am going to do away with the binary data, I would still like to know if this garbage is still showing or was it because of the code handling just one type of image. If possible, kindly check it once again since I found did not find any garbage appended to the end of the binary image data.

What's the current code?

There are three valid ways for delivering images outside of the document root:

through the already mentioned sendfile mechanism.
by serving the raw image data (without any HTML!) with the right Content-Type header
by embedding the image data into a data URL of an img element

1)

<?php

const RESOURCES_PATH = '/www-resources';
const FILENAME_PATTERN = '/\\A\w+\\z/';
const VALID_TYPES = ['image/jpeg', 'image/png'];



// test
$image_filename = 'cat';
$image_type = 'image/jpeg';

// validate image name to prevent header manipulations
if (!preg_match(FILENAME_PATTERN, $image_filename))
{
    echo 'Invalid image.';
    error_log('Unexpected image name when trying to display image #1234.');
    http_response_code(400);
    exit;
}

// validate type to prevent code execution
if (!in_array($image_type, VALID_TYPES, true))
{
    echo 'Invalid image.';
    error_log('Unexpected MIME type when trying to display image #1234.');
    http_response_code(400);
    exit;
}



header('Content-Type: '.$image_type);
header('X-Accel-Redirect: '.RESOURCES_PATH.'/'.$image_filename);    // this is for nginx; Apache has a different header

2)

<?php

const RESOURCES_DIR = '/var/www-resources';
const FILENAME_PATTERN = '/\\A\w+\\z/';
const VALID_TYPES = ['image/jpeg', 'image/png'];



// test
$image_filename = 'cat';
$image_type = 'image/jpeg';

// validate image name to prevent path traversals
if (!preg_match(FILENAME_PATTERN, $image_filename))
{
    echo 'Invalid image.';
    error_log('Unexpected image name when trying to display image #1234.');
    http_response_code(400);
    exit;
}

// validate type to prevent code execution
if (!in_array($image_type, VALID_TYPES, true))
{
    echo 'Invalid image.';
    error_log('Unexpected MIME type when trying to display image #1234.');
    http_response_code(400);
    exit;
}

header('Content-Type: '.$image_type);
// TODO: set cache headers for performance

readfile(RESOURCES_DIR.'/'.$image_filename);

3)

<?php

const RESOURCES_DIR = '/var/www-resources';
const FILENAME_PATTERN = '/\\A\w+\\z/';
const VALID_TYPES = ['image/jpeg', 'image/png'];



function generate_img_data_url($data, $type)
{
    return 'data:'.$type.';base64,'.base64_encode($data);
}



// test
$image_filename = 'cat';
$image_type = 'image/jpeg';

// validate image name to prevent path traversals
if (!preg_match(FILENAME_PATTERN, $image_filename))
{
    echo 'Invalid image.';
    error_log('Unexpected image name when trying to display image #1234.');
    http_response_code(400);
    exit;
}

// validate type to prevent code execution
if (!in_array($image_type, VALID_TYPES, true))
{
    echo 'Invalid image.';
    error_log('Unexpected MIME type when trying to display image #1234.');
    http_response_code(400);
    exit;
}

$image_data = file_get_contents(RESOURCES_DIR.'/'.$image_filename);

?>
<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="utf-8">
        <title>Embedded image</title>
    </head>
    <body>
        <img src="<?= htmlspecialchars(generate_img_data_url($image_data, $image_type), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8') ?>" alt="cat">
    </body>
</html>

As I already said, I strongly prefer the first option.

If I was to use the image blob for displaying the image in view, would I need to validate the binary data? what sort of validation would be needed here ???

My point is that you should validate the MIME type both for incoming images (during upload) and outgoing images (when the files are viewed), because the type might have been manipulated in the meantime.

ajoo · January 3, 2017

Wow !

What's the current code?

I am not sure I understand what you mean? If you are referring to the code for which I requested you to check the image binary data , then it's the same as in #6 of these messages.

That kind of answers almost all of my queries. I will go through and try to ingest all the methods you mentioned in your reply. I will do the required bit with the mime type validation.

Thanks loads Guru Jacques !!

ajoo · January 3, 2017

Hi,

Is it possible to install the xsendfile module on localhost? If so, then how ? I can find no was to do so on the internet. Please help.

Thanks.

Jacques1 · January 3, 2017

The official website has binaries for Windows (.so files) which you should be able to include like all other modules.

In the long run, you should set up a proper development environment with the same operating system as on your server (some Linux distribution, I guess). You can easily do this on top of Windows when you use a virtual machine.

ajoo · January 3, 2017

hmm, that's a great tip. I had no idea i could do that. I wish I had know this earlier, I could have had a true copy of the project on my machine with folders hierarchy exactly as I wanted.

Will do it first now.

Would you please suggest which flavour of the distribution should I go ahead with ? I have a 32 bit windows machine running windows 7. I have to setup a new server in any case, so if you can make a suggestion i will be obliged.

Thanks tons.

benanamen · January 3, 2017

You should look into Vagrant. https://www.vagrantup.com/

It is very easy to use. There are numerous ready to go OS's you can use. It's a "Change your developing life" application. I wish I had used it sooner.

Edited January 3, 2017 by benanamen

ajoo · January 3, 2017

Hi Benanamen,

Sure I will look it up now. Ya I too wish I had know about this concept sooner. Still better late than never !

Thanks very much.

ajoo · January 8, 2017

Hi all !

Installing xsendfile module in http.conf like this just below ... LoadModule xml2enc_module modules/mod_xml2enc.so,

like this

LoadModule xsendfile_module modules/mod_xsendfile.so
XSendFile on
XSendFilePath D:/xampp/htdocs/xampp

gives the following errors in the errors log.

[sun Jan 08 10:39:51.404665 2017] [mpm_winnt:notice] [pid 2636:tid 228] AH00418: Parent: Created child process 3136

[sun Jan 08 10:39:52.314717 2017] [mpm_winnt:notice] [pid 3136:tid 240] AH00354: Child: Starting 150 worker threads.

if I comment out the xsendfile module ( All 3 lines added to httpd.conf ) apache resumes normal service.

Any pointers to get this going.

Thanks loads !

Jacques1 · January 8, 2017

Those aren't errors. What's the actual problem? Are you sure you've downloaded the right “.so” file for your Apache version?

And like I said, this is going to be tricky on Windows. If at all, you should install the module on your new Debian VM.

ajoo · January 8, 2017

@Guru Jacques

I downloaded the Win32 binaries: mod_xsendfile-0.12.zip from https://tn123.org/mod_xsendfile/ ( the link provided by you ) The version of the module works for Apache 2.2. Mine is 2.4.12. I could not find a 2.4.x version.

Immediately after the change, when I start apache, I get

11:23:01 AM [Apache] Attempting to start Apache app...

and it kind of stays there and is probably stopped because against apache on the xampp panel, the button is START.

So if I press this START again, I get :

11:23:01 AM [Apache] Attempting to start Apache app...

11:26:12 AM [Apache] Attempting to start Apache app...

11:26:13 AM [Apache] Status change detected: running

11:26:13 AM [Apache] Status change detected: stopped

11:26:13 AM [Apache] Error: Apache shutdown unexpectedly.

11:26:13 AM [Apache] This may be due to a blocked port, missing dependencies,

11:26:13 AM [Apache] improper privileges, a crash, or a shutdown by another method.

11:26:13 AM [Apache] Press the Logs button to view error logs and check

11:26:13 AM [Apache] the Windows Event Viewer for more clues

11:26:13 AM [Apache] If you need more help, copy and post this

11:26:13 AM [Apache] entire log window on the forums

And like I said, this is going to be tricky on Windows. If at all, you should install the module on your new Debian VM.

:thumb-up: , will do that. If possible, I had wanted to complete the file retrieval before installing the VM and shifting the project there.

I will have to do quite some bit of work with some of my paths , especially for the external folders that I have placed far up the root. :sweat:

Thanks very much !

ajoo · July 8, 2017

Hi all !

Notwithstanding the security issues, I am just trying to get this little snippet, below, to display an image without success, just to see the working of xsendfile. I want to display the file in the browser but the file simply downloads.

<?php
	print_r(apache_get_modules());
	echo "<br><br>";
	echo "test";

	$path = "/vagrant/club/images/pattern1.png";
	$image = "pattern1.png";
	$mime = "image/png";

	
	if(isLoggedIn())
	{
		header('X-Sendfile: '.$path);
		header('Content-Type: '.$mime);
		header('Content-Disposition: attachment; filename="'.$path.'/'.$image.'"');
		exit;
	}

	
	function isLoggedIn(){
		return true;
	}
?>
<h1>Permission denied</h1>
<p>Login first!</p>

and I get the page displays :

Not Found
The requested URL /index.php was not found on this server.
Apache/2.4.7 (Ubuntu) Server at mydemoclub.com Port 80

If I omit the filename from the $path, - (which I think is how it should be used) -, i.e. if I remove pattern1.png from the $path, the page displays the same as above and additionally the error log says.

[sat Jul 08 02:23:07.341682 2017] [:error] [pid 2805] (20024)The given path is misformatted or contained invalid characters: [client 192.168.43.23:63097] xsendfile: not a file /vagrant/club/images

I am doing this on a VirtualBox VPS. The xsendfile module is installed. If I just comment out

	if(isLoggedIn())
	{
		header('X-Sendfile: '.$path);
		header('Content-Type: '.$mime);
		header('Content-Disposition: attachment; filename="'.$path.'/'.$image.'"');
		exit;
	}

What's going wrong here? Please help.

Thanks loads !

Jacques1 · July 8, 2017

None of this makes a lot of sense.

You're delivering the image as an attachment, which specially tells the browser to only download the file and not display it, but at the same time you do want the browser to display the image. You cannot have both.

Then you've removed the filename from the path. How is the webserver supposed to find the file?

ajoo · July 10, 2017

Thanks Guru Jacques for the reply.

With a bit of googling & tweaking this is working. For security I'll use the code that you suggested in #9 of your reply to test the mime type and regex for valid file names.

function isLoggedIn is but a dummy. Is hope this now ok.

<?php
	$path = "/vagrant/club/images";
	$image = "pattern1.png";
	$mime = "image/png";

	
	if(isLoggedIn())
	{
		header('X-Sendfile: '.$path.'/'.$image);
		header('Content-Type: '.$mime);
		header('Content-Disposition: inline; filename="'.$path.'/'.$image.'"');
		exit;
	}

	
	function isLoggedIn(){
		return true;
	}
?>
<h1>Permission denied</h1>
<p>Login first!</p>

Thank you !

Edited July 10, 2017 by ajoo

Jacques1 · July 10, 2017

Handing out the server(!) path as a filename for the user is dangerous, doesn't make sense and is probably not even valid. The filename parameter of the Content-Disposition header expects a filename, not a path.

ajoo · July 10, 2017

Hi Guru Jacques,

Handing out the server(!) path as a filename for the user is dangerous, doesn't make sense and is probably not even valid.

you mean

header('Content-Disposition: inline; filename="'.$path.'/'.$image.'"');

should be

header('Content-Disposition: inline; filename="'.$image.'"');

To be very honest I am not even sure what content-disposition etc means. I am not even sure if this line is, in fact, required to display the image since when I comment it out, the image still displays perfectly. Maybe you can tell me what this Content Disposition header does and if it is in fact not needed.

Thank you.

Jacques1 · July 10, 2017

Don't use code when you don't know what it means.

The purpose of the original header (the one with attachment) was to force the browser to download the file rather than try to display it. Since you apparently never wanted that, the header made no sense.

Now the only effect is that when the user tries to save the file, the default name will be whatever you've put into the filename parameter. At least theoretically. If you want this feature, you'll have to actually test it in different browsers to see if they support it.

ajoo · July 10, 2017

Don't use code when you don't know what it means.

:thumb-up: ! I intended to read about it but I was more keen on getting that image to display on the screen. ( to check its usage ). This is all the functionality that I want.

Thanks for telling the usage. So I can just use the 1st 2 headers and be done with it.

Thanks loads !

Sign In

images upload safely !?

Recommended Posts

Link to comment

Share on other sites

Top Posters In This Topic

Popular Days

Top Posters In This Topic

Popular Days

Popular Posts

Jacques1

ginerjm

Jacques1

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information