Jump to content

images upload safely !?


ajoo

Recommended Posts

Hi Guru Jacques, 

 

The following code display the image correctly, within the div pic :

<div id="box"> 
				<input type='file' id="file" name="image" /><br>	
				<div id="pic">

					<?php if(isset($blob)):
						
					//	header("Content-Type: " .$mime);
					//	header('X-Sendfile: '.$uploadfile);
						
						 echo $prestr.$disp_blob.$poststr;
					

						else :
					?>
						<img id="image" src="#" alt="load_image" /><br>	
					<?php endif; ?>
				</div>
.
.
.
</div>

If, however, I comment out 

echo $prestr.$disp_blob.$poststr;

and un-comment the X-Sendfile headers, the image is displayed centered in the screen, but is not constrained within the pic div as I would like it to be. Surely I must be making a mistake somewhere but there is very little information on the net about X-Sendfile, let alone its usage. Kindly help.

 

Thanks loads ! 

Link to comment
Share on other sites

  • Replies 56
  • Created
  • Last Reply

Why are you trying to use X-Sendfile?

 

X-Sendfile can be used to return files that are not under the webroot, but you can also just use PHP to do the same. You store the files somewhere outside of the webroot and use PHP's file handling routines to get the image data as needed.

 

Your question demonstrates a misunderstanding of how a web page works. Use a web developer tool too look at requests for a page with an img tag in it. You will see many different requests initiated by the browser for the individual components of a page (.js, .css, images).

 

Your html page has nothing but html tags in it. The image data must be returned in a separate request.

 

The standard way of doing something like that is to have a script that returns the image data, with the proper headers set.

 

// getimg.php

<?php

    $id = isset($_GET['id']) ? (int)$_GET['id'] : 0;

    if ($id > 0) {
       //Locate the image, determine mime-type
    } else {
       // use some default error image
    }
    $mimetype = exif_imagetype($image);
    header("Content-type: " . image_type_to_mime_type($mimetype));
    // Return the file or perhaps use X-Sendfile
Then in your html file you would be using:

 

<div id="mainImage">
    <img src="getimg.php?id=33">
</div>
Link to comment
Share on other sites

Why are you trying to use X-Sendfile?

 

Because it's the only safe and sane way to serve user-provided files.

 

 

 

X-Sendfile can be used to return files that are not under the webroot, but you can also just use PHP to do the same.

 

You mean those scripts where you can just inject a few “..” into the path and then download arbitrary files from the server? ;)

 

This is far too critical for any homemade solutions. In fact, storing the files outside of the directory root for security reasons and then compromising security with a flaky download script really defeats the whole purpose.

 

Efficiency is another big issue of (naive) PHP implementations.

  • X-Sendfile supports caching by default. With a PHP script, you either have to live with the fact that the interpreter is invoked for every single file request, or you have to reimplement those parts of the Apache module.
  • Range requests are also supported out of the box. With a PHP script, you either can't have that, or you have to reimplement those parts of the Apache module as well.

A simple PHP script may be good enough for a quick test in a local test environment, but I definitely wouldn't recommend this for any production site.

 

I'm actually surprised that Apache still hasn't “officially” integrated this feature like nginx did with X-Accel.

Link to comment
Share on other sites

You make some good points, although the dangers you discuss can obviously be mitigated with file systems permissions.

 

The X-Sendfile has other additional performance advantages, but X-Sendfile doesn't change the basic rules of how browsers and web pages work. He still is going to need the script I described, even if he uses X-Sendfile in it, unless this app is fetching raw files, with no associated information fr0m a database. Or, the target

 

I did scan this thread, so I may be missing something.

Link to comment
Share on other sites

There's no doubt that ajoo misunderstands how file serving works, so, yes, that needs to be fixed. But I'm not too worried about that, because you already explained it, he has a working implementation which he just needs to adjust, and there are no less than three example scripts in #9.

 

I was talking specifically about why X-Sendfile is important and why it shouldn't be replaced with a simple readfile() script. Manual path validation, file permissions etc. are all great, but people tend to get them wrong a lot. The nice thing about X-Sendfile is that it's simple and can act as a last line of defense even when everything else failed.

Link to comment
Share on other sites

Hi Guru Jacques, Gizmola ! 

 

Thanks for the response. I have been trying to get this going but without success. Please show me how it can be done. 

 

Coming down the php file I have opened a graphic file and saved it in the img folder on the server. The $uploadfile contains the complete path to the image, including the complete image name. 

<div id="box">
                <input type='file' id="file" name="image" /><br>    
                <div id="pic">

                    <?php if(isset($blob)):
                        
                    header("Content-Type: " .$mime);
                    echo "<img src='".header('X-Sendfile: '.$uploadfile)."'>";
                        
                        else :
                    ?>
                        <img id="image" src="#" alt="load_image" /><br>    
                    <?php endif; ?>
                </div>
.
.
.
</div>

This is the last I tried which is also clearly wrong. While I can use xsendfile to display the image using the $uploadfile, I simply cannot confine it to a div. I have used the image tag in the code above but obviously that is not how this will function. 

 

Here is what I get when I inspect the page in chrome :

 

 

<head>

<meta name="viewport" content="width=device-width, minimum-scale=0.1">

<title>upload_pics.com (150×200)</title>

</head>

<body style="margin: 0px; background: #0e0e0e;">

<img style="-webkit-user-select: none;background-position: 0px 0px, 10px 10px;background-size: 20px 20px;background-image:linear-gradient(45deg, #eee 25%, transparent 25%, transparent 75%, #eee 75%, #eee 100%),linear-gradient(45deg, #eee 25%, white 25%, white 75%, #eee 75%, #eee 100%);" src="http://upload_pics.com/#" width="150" height="200"></body>

 

Seems to have generated its own html file.

 

As Guru Jacques mentioned 

 

.... he has a working implementation which he just needs to adjust, and there are no less than three example scripts in #9.

 

It may be a small tweak but I am unable to figure it out. Please enlighten !!  

 

Thanks loads !

Link to comment
Share on other sites

The purpose of the X-Sendfile header is to make the webserver send a response with the content of a single file. You cannot do that in the middle of an HTML document, because when you've already generated HTML markup, that implies you want to send an HTML response. Both at the same time isn't possible. A response cannot both be an HTML document and, say, an image.

 

As gizmola already said, you need two scripts. One script parses its URL parameters, looks up the corresponding image and then serves that image with X-Sendfile. It doesn't do anything else. There's no HTML markup anywhere. You should already have this script, or simply use the first one from #9. Then you need a second script. This one generates an HTML page with an embedded image. But it does not (and cannot) use X-Sendfile. It needs to put the URL of the first script into the src attribute of an image element:

<div id=box>
...
     <img src="/path/to/xsendfile_script.php?id=1234" alt="...">
...
</div>

Again, there are two separate script for two different tasks.

Link to comment
Share on other sites

Hi Guru Jacques,

 

While using XSendfile, Is it presupposed & required that the file be stored on the server and be referenced from a database ? 

 

If so, then in the scenario where a  user is filling a form and which has an image besides some data fields, if there is an error on submission then user input data-is retained and displayed from the post variables, it would not be possible to use XSendfile to refresh the image field as well. Is that  correct ?

 

Thank you. 

Link to comment
Share on other sites

While using XSendfile, Is it presupposed & required that the file be stored on the server and be referenced from a database ? 

 

The file must be on the server, but X-Sendfile doesn't know anything about databases. Where the path comes from is irrelevant.

 

 

 

If so, then in the scenario where a  user is filling a form and which has an image besides some data fields, if there is an error on submission then user input data-is retained and displayed from the post variables, it would not be possible to use XSendfile to refresh the image field as well. Is that  correct ?

 

So you want some kind of preview for images that haven't been stored at their final location yet?

 

This is technically possible, but it's probably a bad idea. If add a temporary folder like the upload directory to the X-Sendfile whitelist, you make it easier for attackers to abuse the mechanism. Temporary files are also inherently more dangerous, because they may not have been validated completely.

 

Of course you could implement some kind of "safe" temporary directory which only holds files which have been fully validated. But that's going to be complex.

 

If you want to allow the user to reuse a previously uploaded file, I suggest you simply add a message that shows the file info (name, size etc.). You're going to need a hidden field for a file reference. Don't use the original temporary filename, though, because this may be weak and allow users to "steal" temporary files from others. Generate a purely random name.

Link to comment
Share on other sites

Hi Guru Jacques !

 

 

 

So you want some kind of preview for images that haven't been stored at their final location yet?

 

Yes, I already have a preview and a working form. I finally came back to it to replace storing the image data into the DB, as was advised by you earlier, with files on the server and using XSendfile to display them. I thought maybe the preview , that's being created using the image data as of now, could be replaced by XSendfile and that was what I was trying to achieve till I realized that XSendfile works only with files that have been stored. That's what I confirmed from you in my last question.

 

What I have finally thought and also tried out roughly is to use the image data for the preview since that is relatively simple. Once the form is approved and submitted, the image file is stored in the server outside of the web-root with a random name and the filename and image type is stored in the DB. Now if the form is retrieved, say for any purpose like editing etc. the image file is served by XSendfile for all subsequent purposes. Do you think that would be good enough !?

 

I'll post the modified code soon after refining it the best I can.

 

Thanks loads !

Link to comment
Share on other sites

Hi Guru Jacques !

 

The working example: (I have not used the image blob here ).  I have commented out the filename checking that uses the FILE_PATTERN constant that you gave in your example since i do not know exactly what it does and why and it gave an error with the generated filename. Please shed some light on that. I hope the rest of the code is Ok. (In the actual code I will use bin2hex in place of substr(hash...) Maybe you  created the regex to work with bin2hex. 

 

Kindly take a look.   

 

 Create Table : 

CREATE TABLE `images` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `name` varchar(60) COLLATE utf8_unicode_ci NOT NULL,
  `original_name` varchar(60) COLLATE utf8_unicode_ci NOT NULL,
  `mime_type` varchar(20) COLLATE utf8_unicode_ci NOT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `name` (`name`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

ups6_phpfreaks_2.php :

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

define("HALF_MB", 512000);	// defines will go into the config file. 
define("SV_WIDTH", 150);
define("SV_HEIGHT", 175); 

$uploaddir = getenv('DIR_IMAGES'); // points to a folder 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'];


	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(empty($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";
		
		if 	(($mime === "image/jpg") || ($mime === "image/jpeg") || ($mime === "image/png") || ($mime === "image/gif"))
		{

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

		}
		
		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);
		
		if($mime === 'image/png') $imagemoved = imagepng($dst,$uploadfile);
		if($mime === 'image/jpg' || $mime === 'image/jpeg') $imagemoved = imagejpeg($dst,$uploadfile);
		if($mime === 'image/gif') $imagemoved = imagegif($dst,$uploadfile);
		imagedestroy($src);
		imagedestroy($dst);
		
		if ($imagemoved) 
		{
			try
			{
				$query = "INSERT INTO images (name, original_name, mime_type) VALUES (?,?,?)";

				$stmt = $link->prepare($query);
				$stmt->bind_param('sss',$uploadfile,$filename,$mime);
				if($stmt->execute()){
					
					$id=$stmt->insert_id;	// get the just inserted id
					$rec_insert = 'inserted';

				}
				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>";

}

function fcheckMimeType($mimetype)
{
	$mimes	= array('image/gif','image/jpeg','image/jpg','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">
				
			<?php 
				if(isset($id) && $id !== 0){
						
					echo "<img src='ups6_view_2.php?id=$id' />";	// Displays Image after upload
					
				}else{

					echo "<img id='image' src='#' alt='load image' />";	// Selected image loads here.
				}		
			?>
			
			</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>

ups6_view_2

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

const FILENAME_PATTERN = '/\\A\w+\\z/';

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

$id = $_GET['id'];


	if(!is_numeric($id)) 
	{
		die("Bad File Index");
	}

	$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();
/*	
// validate image name to prevent header manipulations
	if (!preg_match(FILENAME_PATTERN, $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(!fcheckMimeType($type))
{
    echo 'Invalid image.';
    error_log('Unexpected MIME type when trying to display image #1234.');
    http_response_code(400);
    exit;
}

	header("Content-Type: " .$type);
	header('X-Sendfile: '.$filename);

?>

Thanks load !!!

Link to comment
Share on other sites

Several parts of your code don't make much sense.

  • The lengths of the database table fields are off. MIME types can be up to 255 characters long, so a VARCHAR(20) is overly optimistic. Even some of the officially registered image types are longer than that. In any case: When you have tight length restrictions, make sure the database system is running in strict mode. Otherwise MySQL may silently truncate long values instead of rejecting them.
     
  • Why are you using an environment variable to store the upload directory? What's the purpose of that? Just put the path into a configuration file.
     
  • Server-side upload errors must be logged. If the temporary directory is missing, or if PHP cannot write the uploaded data to disk, or if some extension interferes the upload mechanism, you definitely need to know.
     
  • The whole image scaling code looks wrong. This:
    if($widthscale < 1) $nwidth = $ix*$widthscale;
    
    makes images which are already small enough even smaller. And this:
    $widthscale = $ix/SV_WIDTH;
    $nwidth = $ix/$widthscale;
    
    is a really complicated and erroneous way of saying
    $nwidth = SV_WIDTH;
    
    And since you don't preserve the image's aspect ratio, the result should look pretty bad.
     
  • You're doing this strange duplicate check when you insert the image. What kind of duplicates do you expect? The only UNIQUE column is name, but that's the random filename you've generated yourself. The random number generator should never produce duplicate values, and even if it somehow does, what is the user supposed to do with this information? Remove the entire try statement.
     
  • For some reason, you store the full file path in the database. This is not only redundant (the upload directory should always be the same), it's also dangerous. If you aren't careful, you can end up trying to serve files from arbitrary locations – you're actually doing that right now.
     
  • The MIME type check doesn't make sense:
    $testmime = filter_var(in_array($mimetype,$mimes),FILTER_VALIDATE_BOOLEAN)? TRUE : FALSE;
    

    What is this line supposed to do? The return value of the in_array() function already is a boolean. You cannot make it “more boolean”. Use the code I gave you. And include the function from an external script instead of duplicating the code.
     

  • The is_numeric() function doesn't do what you think it does. If you check the manual, you'll see that it also accepts expressions like “+0123.45e6”, and I'm pretty sure you don't consider this valid input. To validate the ID, use ctype_digit() which only accepts a sequence of decimal digits.
     

  • Yes, the FILE_PATTERN check is necessary. If you don't validate $filename, this may be used to perform an HTTP header injection or load arbitrary files from your server. Fix your random number generator, store only the actual filename in your database, then the check will work just fine.
Link to comment
Share on other sites

Wow !! What a great answer / code review Guru Jacques !!

 

 

 

The lengths of the database table fields are off.

 Ok. So should i  let them be 255 in that case ?

 

 

 

Why are you using an environment variable to store the upload directory?

Well I saw a number of examples where they used this setting in the conf file of the server. I'll shift that to a config file.

 

 

 

Server-side upload errors must be logged.

 

hmmm I thought php logged all those errors. No ?

 

 

 

The whole image scaling code looks wrong

 

 I know there was an issue with it since it made the smaller images even smaller but I did not get a chance to look into it. I'll debug it. 

 

 

 

And since you don't preserve the image's aspect ratio, ...

 

In practise, this shouldn't be an issue since the images uploaded would all be passport size and uploaded by logged in persons only. 

 

 

 

You're doing this strange duplicate check when you insert the image. What kind of duplicates do you expect?

Yes that was just in case a the random file name produces a duplicate. I'll remove it. But should I still not check for the duplicate name and programatically generate a new one without any messages being shown? Oh I think that you think it's redundant because that would never happen !?

 

I'll look into all the other issues too and revert. 

 

Thanks loads again for the super reply !!

Link to comment
Share on other sites

Ok. So should i  let them be 255 in that case ?

 

Yes. Or any other realistic number.

 

Using small VARCHAR limits doesn't save space, if that's what you're trying to do. The same string takes up the exact same number of bytes, regardless of whether you have a VARCHAR(20), VARCHAR(100) or VARCHAR(255). Only when you go beyond 255, there will be one(!) additional byte for the size.

 

 

 

hmmm I thought php logged all those errors. No ?

 

No. Why should it? PHP logs exceptions and PHP errors. Handling upload problems is up to you.

 

 

 

In practise, this shouldn't be an issue since the images uploaded would all be passport size and uploaded by logged in persons only.

 

It still doesn't make sense to mess up the pictures. The standard way is to scale the image until it fits into the box.

 

 

 

Yes that was just in case a the random file name produces a duplicate. I'll remove it. But should I still not check for the duplicate name and programatically generate a new one without any messages being shown? Oh I think that you think it's redundant because that would never happen !?

 

You might as well prepare for getting attacked by a polar bear in the next 5 minutes.

Link to comment
Share on other sites

 

 

You might as well prepare for getting attacked by a polar bear in the next 5 minutes. 

:lol:  :lol:  :lol:  :thumb-up:

 

 

For some reason, you store the full file path in the database. This is not only redundant (the upload directory should always be the same), it's also dangerous. If you aren't careful, you can end up trying to serve files from arbitrary locations – you're actually doing that right now.

 

Having removed the path from the filename and storing just the filename in the database I have run into this very peculiar issue. As an example, I was storing the filename, as pointed by you with the path, /vagrant/images/some_random_name. In this case the XSendfile worked fine. The image is displayed correctly after the upload button is clicked. 

 

Now I removed the path name from the file, stored the pathname as $uploaddir = '/vagrant/images';  The name of the image is stored in the database as

some_random_name. When I concatenate the two to create the full path ( ditto as before) to the file as $filename = $uploaddir.'/'.$filename;  the image fails to display when the upload button is pressed.  

 

and I get the following error in the logs :

 

 

[sat Jul 22 13:28:39.729100 2017] [:error] [pid 3268] (2)No such file or directory: [client 192.168.2.9:50232] xsendfile: cannot open file:/vagrant/upload_security/3ff698c417d97d1e4508867c, referer: http://upload_security.com/

 
The root path of the server is /vagrant/upload_security and the XSendFilePath is set to /vagrant/images/
 
I have no idea why this is happening. Please advise. 
 
Thanks loads !
Link to comment
Share on other sites

var_dump() the actual $filename before you set the headers. The only possible reason for why you get a wrong path is that you've either supplied a relative file path (i. e. $uploaddir is empty) or a wrong absolute path.

 

Originally, your $uploaddir came from an environment variable, and I assume you've changed that. Are you sure the new configuration mechanism actually works? This sounds a lot like $uploaddir simply isn't set at all.

Link to comment
Share on other sites

hmm I think because it's called through an image tag ( <IMG src = .. >, echo or var_dump does not display anything at all.  Is there any other way to get the output ?

 

Ok so I made a small hack and these are the outputs below: 

 

string(15) "/vagrant/images"   ------------------------------------------------------------ $uploaddir 
string(40) "/vagrant/images/fe2becf2e9626383407e99af" ----------------------------$filename =  $uploaddir.'/'.$filename.

Link to comment
Share on other sites

Here's the code :

 

ups6_phpfreaks_3.php

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

define("HALF_MB", 512000);	// defines will go into the config file. 
define("SV_WIDTH", 150);
define("SV_HEIGHT", 175); 
define("FILE_NAME_LENGHT", 12);
require_once '/vagrant/upload_security/lib/random_compat/random.php';

$uploaddir = "/vagrant/images"; // 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'];


	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(empty($error_msg))		// no errors flagged
	{
	
		$random_filename = bin2hex(random_bytes(FILE_NAME_LENGHT));
		$uploadfile = $uploaddir.'/'.$random_filename;
	//	echo $uploadfile.'<br>';
		
		if 	(($mime === "image/jpg") || ($mime === "image/jpeg") || ($mime === "image/png") || ($mime === "image/gif"))
		{

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

		}
		
		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 = SV_WIDTH;	// what was I thinking !???? 

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

		$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($mime === 'image/png') $imagemoved = imagepng($dst,$uploadfile);
		if($mime === 'image/jpg' || $mime === 'image/jpeg') $imagemoved = imagejpeg($dst,$uploadfile);
		if($mime === 'image/gif') $imagemoved = imagegif($dst,$uploadfile);
		imagedestroy($src);
		imagedestroy($dst);
		
		if ($imagemoved) 
		{

			$query = "INSERT INTO images (name, original_name, mime_type) VALUES (?,?,?)";

				$stmt = $link->prepare($query);
				$stmt->bind_param('sss',$random_filename,$filename,$mime);	// I can do quit saving image blob
				if($stmt->execute()){
					
					$id=$stmt->insert_id;	// get the just inserted id
					$rec_insert = 'inserted';
				//	echo "ID = ".$id;
				}
		}else echo "Insert failed. Try again Later";	
		
	}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_3.php?id=$id'>here</a>";
	if($rec_insert === 'failed') echo "Record could not be inserted";
	
}

function fcheckMimeType($mimetype)
{
	$mimes	= array('image/gif','image/jpeg','image/jpg','image/png');
	$testmime = filter_var(in_array($mimetype,$mimes),FILTER_VALIDATE_BOOLEAN);
	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">
				
			<?php 
			/*	if(isset($id) && $id !== 0){
						
					echo "<img src='ups6_view_2.php?id=$id' />";	
					
				}else{

					echo "<img id='image' src='#' alt='load image' />";		// This displays the image.
				}		
			*/	
			
			echo "<img id='image' src='#' alt='load 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>

ups6_view_3.php :

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

const FILENAME_PATTERN = '/\\A\w+\\z/';
$uploaddir = "/vagrant/images"; // Outside of web root

function fcheckMimeType($mimetype)
{
	$mimes	= array('image/gif','image/jpeg','image/jpg','image/png');
	$testmime = filter_var(in_array($mimetype,$mimes),FILTER_VALIDATE_BOOLEAN);
	return $testmime;
}

function fcheckNumber($num){

$num = (String)$num;
// just for readability
define('PHP_INT_MIN', 0);

	if (ctype_digit($num) && $num >= PHP_INT_MIN && $num <= PHP_INT_MAX)
	{
		// echo $num;
		return $num;
	}
	else
	{
		return false;
		// number either not numeric or out of range
	}	
}


$id = $_GET['id'];


	if(!fcheckNumber($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();
/*	
// validate image name to prevent header manipulations
	if (!preg_match(FILENAME_PATTERN, $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(!fcheckMimeType($type))
{
    echo 'Invalid image.';
    error_log('Unexpected MIME type when trying to display image #1234.');
    http_response_code(400);
    exit;
}

	$filename = $uploaddir.'/'.$filename;
 	var_dump($uploaddir);
	echo "<br>";
	var_dump($filename);
	exit;
	
	header("Content-Type: " .$type);
	header('X-Sendfile: '.$filename);

?>

Thanks !

Link to comment
Share on other sites

  • 2 weeks later...

Hi, 

 

Please may I request anyone to try out the code in #45, which uses a hard coded path in line 12 

$uploaddir = "/vagrant/images"; // Outside of web root

instead of using an environment variable as used in #38 as

$image_path = getenv("DIR_IMAGES");

and confirm if they get the same surprising result as I do. The first one fails while the 2nd one works though they both should work just fine. 

 

Thanks all !

 

Link to comment
Share on other sites

Why don't you debug this? If the first path fails, then it's simply the wrong path. This has nothing to do with hard-coded strings. A string is a string.

 

So the obvious solution is to compare the strings and find out where they differ.

<?php

var_dump($uploaddir);
var_dump($image_path);

// do a hex dump as well
var_dump( bin2hex($uploaddir) );
var_dump( bin2hex($image_path) );
Link to comment
Share on other sites

Hi Guru Jacques !

 

 

 

Why don't you debug this?

 

Done !!  :thumb-up:  :thumb-up:.  Even though I am still not exactly sure where the problem was since I just deleted the file name creation and storing bit again and finally it worked.

 

Strangely the strings were the same in both cases. I think the mistake lay somewhere during creation of file name and then it's storage.

 

Thanks !

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.


×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.