Jump to content



Photo
- - - - -

Php Problem

php web design upload php image resize

  • Please log in to reply
11 replies to this topic

#1 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 05 July 2012 - 07:21 PM

My PHP upload seems to be having problems. Here is my php:



<?PHP
class ImgResizer {
private $originalFile = '';
public function __construct($originalFile = '') {
$this -> originalFile = $originalFile;
}
public function resize($newWidth, $targetFile) {
if (empty($newWidth) || empty($targetFile)) {
return false;
}
$src = imagecreatefromjpeg($this -> originalFile);
list($width, $height) = getimagesize($this -> originalFile);
$newHeight = ($height / $width) * $newWidth;
$tmp = imagecreatetruecolor($newWidth, $newHeight);
imagecopyresampled($tmp, $src, 0, 0, 0, 0, $newWidth, $newHeight, $width, $height);
if (file_exists($targetFile)) {
unlink($targetFile);
}
imagejpeg($tmp, $targetFile, 85); // 85 is my choice, make it between 0 – 100 for output image quality with 100 being the most luxurious
}
}

$files=array();
$fdata=$_FILES['userfile'];
if(is_array($fdata['name'])){
for($i=0;$i<count($fdata['name']);++$i){
$files[]=array(
'name' =>$fdata['name'][$i],
'type' => $fdata['type'][$i],
'tmp_name'=>$fdata['tmp_name'][$i],
'error' => $fdata['error'][$i],
'size' => $fdata['size'][$i]
);
}
} else $files[]=$fdata;

//begin upload code
$continue = 1;
$adr = "pics/" . $_GET["adr"] . "/";
$maxSize = 1000000000;

foreach ($files as $file) {
if(($file['type'] == "image/gif") || ($file['type'] == "image/jpeg") || ($file['type'] == "image/pjpeg") || ($file['type'] == "image/png")) {
}
else {
$errorMessage = "<p>Wrong File Type</p>";
$continue = 0;
}

if($file['size'] > $maxSize) {
$errorMessage = "<p>File is too large.</p>";
$continue = 0;
}

//check for errors
if(($file['error'] > 0)) {
echo "<p>Error" . $file['error'] . "</p>";
}

elseif($continue == 0) {
echo $errorMessage;
}

//display info
elseif($continue == 1) {
echo "Name:" . $file['name'] . "<br />";
echo "Type:" . $file['type'] . "<br />";

if((file_exists($adr . $file['name']))) {
echo "<p>" . $file['name'] . "already exists!</p>";
}
else {
//upload large version
move_uploaded_file($file['tmp_name'], $adr . $file['name']);

//resize and copy image for thumbnail
$work = new ImgResizer($adr . $file['name']);
$work -> resize(180, $adr . "/thumb/" . $file['name']);


echo "<p>Upload Successful! View file <a href='" . $adr . $file['name'] . "'>here.</a></p>
<p><a href='" . $adr . "/thumb/" . $file['name'] . "'>Thumbnail</a></p>";

}
}
}

?>


My HTML:



<h1>Upload image</h1>
<p>
<span class="notification">Use shift+click or ctrl+click to select multiple images at once. <br /></span>
</p>

<p>Only <span class="emp">*.png, *.jpeg, *.jpg, and *.gif</span> filetypes are allowed.<br /></p>

<form action= "<?PHP echo "upload.php?adr=" . $_POST["formFeast"]; ?>" method="post" enctype="multipart/form-data">
<label for="file">Filename:</label>
<input type="file" name="userfile[]" id="userfile" multiple="multiple" />
<br />
<input type="submit" name="submit" value="Submit" />
</form>
<p><a href="admin-dashboard">Back to admin dashboard</a></p>




Most of the time it works great. However, when uploading lots of files at the same time, I get "Wrong File Type" as an output. and only 1 "Wrong file type", not multiple.

Does this have to do with GD?

#2 2072

2072

    Casio over god

  • Admin
  • PipPipPipPipPipPipPipPip
  • 1565 posts
  • Gender:Male
  • Location:Somewherebourg
  • Interests:Alternative states of consciousness, programming, making things work the best they possibly can.

  • Calculators:
    AFX2 ROM 1.02, CFX-9940GT+, FX-180P-Plus

Posted 05 July 2012 - 07:45 PM

This has to do with your test here:


if(($file['type'] == "image/gif") || ($file['type'] == "image/jpeg") || ($file['type'] == "image/pjpeg") || ($file['type'] == "image/png")) {
    }
    else {
$errorMessage = "<p>Wrong File Type</p>";
$continue = 0;
    }

Just replace the error message by:

$errorMessage = "<p>Wrong File Type: '$file[type]'</p>";

then you'll know a little bit more about what's happening

#3 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 05 July 2012 - 07:53 PM

New code:


<?PHP
class ImgResizer {
private $originalFile = '';
public function __construct($originalFile = '') {
$this -> originalFile = $originalFile;
}
public function resize($newWidth, $targetFile) {
if (empty($newWidth) || empty($targetFile)) {
return false;
}
$src = imagecreatefromjpeg($this -> originalFile);
list($width, $height) = getimagesize($this -> originalFile);
$newHeight = ($height / $width) * $newWidth;
$tmp = imagecreatetruecolor($newWidth, $newHeight);
imagecopyresampled($tmp, $src, 0, 0, 0, 0, $newWidth, $newHeight, $width, $height);
if (file_exists($targetFile)) {
unlink($targetFile);
}
imagejpeg($tmp, $targetFile, 85); // 85 is my choice, make it between 0 – 100 for output image quality with 100 being the most luxurious
}
}

$files=array();
$fdata=$_FILES['userfile'];
if(is_array($fdata['name'])){
for($i=0;$i<count($fdata['name']);++$i){
$files[]=array(
'name' =>$fdata['name'][$i],
'type' => $fdata['type'][$i],
'tmp_name'=>$fdata['tmp_name'][$i],
'error' => $fdata['error'][$i],
'size' => $fdata['size'][$i]
);
}
} else $files[]=$fdata;

//begin upload code
$continue = 1;
$adr = "pics/" . $_GET["adr"] . "/";
$maxSize = 1000000000;

foreach ($files as $file) {
if(($file['type'] == "image/gif") || ($file['type'] == "image/jpeg") || ($file['type'] == "image/pjpeg") || ($file['type'] == "image/png")) {
}
else {
$errorMessage = "<p>Wrong File Type</p>
<p>Type: " . $file['type'] . "</p>
<p>Name: " . $file['name'] . "</p>";
$continue = 0;
}

if($file['size'] > $maxSize) {
$errorMessage = "<p>File is too large.</p>";
$continue = 0;
}

//check for errors
if(($file['error'] > 0)) {
echo "<p>Error" . $file['error'] . "</p>";
}

elseif($continue == 0) {
echo $errorMessage;
}

//display info
elseif($continue == 1) {
echo "Name:" . $file['name'] . "<br />";
echo "Type:" . $file['type'] . "<br />";

if((file_exists($adr . $file['name']))) {
echo "<p>" . $file['name'] . " already exists!</p>";
}
else {
//upload large version
move_uploaded_file($file['tmp_name'], $adr . $file['name']);

//resize and copy image for thumbnail
$work = new ImgResizer($adr . $file['name']);
$work -> resize(180, $adr . "/thumb/" . $file['name']);


echo "<p>Upload Successful! View file <a href='" . $adr . $file['name'] . "'>here.</a></p>
<p><a href='" . $adr . "/thumb/" . $file['name'] . "'>Thumbnail</a></p>";

}
}
}

?>



Output:


Wrong File Type

Type:

Name:



EDIT:

I appended " . var_export($file, true) " to my error message.



<?PHP
class ImgResizer {
private $originalFile = '';
public function __construct($originalFile = '') {
$this -> originalFile = $originalFile;
}
public function resize($newWidth, $targetFile) {
if (empty($newWidth) || empty($targetFile)) {
return false;
}
$src = imagecreatefromjpeg($this -> originalFile);
list($width, $height) = getimagesize($this -> originalFile);
$newHeight = ($height / $width) * $newWidth;
$tmp = imagecreatetruecolor($newWidth, $newHeight);
imagecopyresampled($tmp, $src, 0, 0, 0, 0, $newWidth, $newHeight, $width, $height);
if (file_exists($targetFile)) {
unlink($targetFile);
}
imagejpeg($tmp, $targetFile, 85); // 85 is my choice, make it between 0 – 100 for output image quality with 100 being the most luxurious
}
}

$files=array();
$fdata=$_FILES['userfile'];
if(is_array($fdata['name'])){
for($i=0;$i<count($fdata['name']);++$i){
$files[]=array(
'name' =>$fdata['name'][$i],
'type' => $fdata['type'][$i],
'tmp_name'=>$fdata['tmp_name'][$i],
'error' => $fdata['error'][$i],
'size' => $fdata['size'][$i]
);
}
} else $files[]=$fdata;

//begin upload code
$continue = 1;
$adr = "pics/" . $_GET["adr"] . "/";
$maxSize = 1000000000;

foreach ($files as $file) {
if(($file['type'] == "image/gif") || ($file['type'] == "image/jpeg") || ($file['type'] == "image/pjpeg") || ($file['type'] == "image/png")) {
}
else {
$errorMessage = "<p>Wrong File Type</p>
<p>Type: " . $file['type'] . "</p>
<p>Name: " . $file['name'] . "</p>
<p>" . var_export($file, true) . "</p>";
$continue = 0;
}

if($file['size'] > $maxSize) {
$errorMessage = "<p>File is too large.</p>";
$continue = 0;
}

//check for errors
if(($file['error'] > 0)) {
echo "<p>Error" . $file['error'] . "</p>";
}

elseif($continue == 0) {
echo $errorMessage;
}

//display info
elseif($continue == 1) {
echo "Name:" . $file['name'] . "<br />";
echo "Type:" . $file['type'] . "<br />";

if((file_exists($adr . $file['name']))) {
echo "<p>" . $file['name'] . " already exists!</p>";
}
else {
//upload large version
move_uploaded_file($file['tmp_name'], $adr . $file['name']);

//resize and copy image for thumbnail
$work = new ImgResizer($adr . $file['name']);
$work -> resize(180, $adr . "/thumb/" . $file['name']);


echo "<p>Upload Successful! View file <a href='" . $adr . $file['name'] . "'>here.</a></p>
<p><a href='" . $adr . "/thumb/" . $file['name'] . "'>Thumbnail</a></p>";

}
}
}

?>




New Output:


Wrong File Type

Type:

Name:

NULL

EDIT2

Output with error_reporting(E_ALL);:


Notice: Undefined index: userfile in /home/flyingfisch/public_html/shj/upload.php on line 44

Wrong File Type

Type:

Name:

NULL


Line 44:

$fdata=$_FILES['userfile'];


#4 MicroPro

MicroPro

    Casio Overlord

  • Deputy
  • PipPipPipPipPipPipPip
  • 640 posts
  • Gender:Male
  • Location:Iran

  • Calculators:
    Casio ClassPad 300

Posted 06 July 2012 - 11:36 AM

So this means the file was not uploaded. How does your uploading HTML form look like? Are you sure your input has the correct "name" attribute set? (<input type="file" name="userfile">)

#5 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 06 July 2012 - 02:02 PM

My HTML looks like this:



<form action= "<?PHP echo "upload.php?adr=" . $_POST["formFeast"]; ?>" method="post" enctype="multipart/form-data">
<label for="file">Filename:</label>
<input type="hidden" name="MAX_FILE_UPLOADS" value="100" />
<input type="hidden" name="MAX_FILE_SIZE" value="100000000000000" />
<input type="file" name="userfile[]" id="userfile" multiple="multiple" />
<br />
<input type="submit" name="submit" value="Submit" />
</form>


#6 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 09 July 2012 - 06:58 PM

I was told by MicroPro that this code was insecure: $adr = "pics/" . $_GET["adr"] . "/";

He said that a dir could be inserted into the url, and that was insecure.

I don't know how that would be, because I have 777 permissions on the two folders that i want to allow public uploading on, and all the rest have 755 permissions.

Could someone tell me if i need to take more precautionary measures, and if so, how?



#7 MicroPro

MicroPro

    Casio Overlord

  • Deputy
  • PipPipPipPipPipPipPip
  • 640 posts
  • Gender:Male
  • Location:Iran

  • Calculators:
    Casio ClassPad 300

Posted 09 July 2012 - 07:27 PM

Sorry I was too busy to explain more :)

It's not about permissions. Guess what happens if someone calls your php page this way. In which folder will the uploaded image be saved?

upload.php?adr=..

Let's think the banner in your site is in "images" folder and is named "banner.jpg". What about uploading an image called "banner.jpg" using a modified form like this to your server?
<form action= "upload.php?adr="../images" ...


#8 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 09 July 2012 - 07:31 PM

Well, ok, but the directories that have 777 permissions are not what i care about. they only have public upload images in them. also, the script checks for same file names.

in other words, i have special directories for image uploads. they have no important images in them. the directories with important images have 755 permissions. So someone with a modified form would get an error if he tried to upload to those directories.

Right?

#9 MicroPro

MicroPro

    Casio Overlord

  • Deputy
  • PipPipPipPipPipPipPip
  • 640 posts
  • Gender:Male
  • Location:Iran

  • Calculators:
    Casio ClassPad 300

Posted 10 July 2012 - 01:58 PM

Look I'm not very good in permissions, this should be tested, but I think 755 and even 644 permissions are not safe enough. You should not rely solely on permissions since besides them there are other configurations too. Normally if you can write to a folder, then your php application can, too, and the hacker there can abuse this.

I also found this and think is useful: http://forums.cpanel...afe-124369.html

This google search lead me to this and this (read all of the answers) and this.

i have special directories for image uploads.

If there is more than one, then you want to make sure users don't put their image in the wrong directory by sending paths containing ".." to your server yeah?

EDIT: Look I'm obviously not a expert on the subject. :P

Edited by MicroPro, 10 July 2012 - 02:11 PM.


#10 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 10 July 2012 - 02:14 PM

...
If there is more than one, then you want to make sure users don't put their image in the wrong directory by sending paths containing ".." to your server yeah?


Well, that's not really an issue since the upload form is protected by a password and only available for me and another guy.

...
I also found this and think is useful: http://forums.cpanel...afe-124369.html

This google search lead me to this and this (read all of the answers) and this.
...


After reading those, I think i am going to restrict what ?adr can equal. I will just make sure that it is in one of the 2 directories. If not... error!

EDIT:

OK, edited my code. Here is my new code:


<?PHP
class ImgResizer {
private $originalFile = '';
public function __construct($originalFile = '') {
$this -> originalFile = $originalFile;
}
public function resize($newWidth, $targetFile) {
if (empty($newWidth) || empty($targetFile)) {
return false;
}
$src = imagecreatefromjpeg($this -> originalFile);
list($width, $height) = getimagesize($this -> originalFile);
$newHeight = ($height / $width) * $newWidth;
$tmp = imagecreatetruecolor($newWidth, $newHeight);
imagecopyresampled($tmp, $src, 0, 0, 0, 0, $newWidth, $newHeight, $width, $height);
if (file_exists($targetFile)) {
unlink($targetFile);
}
imagejpeg($tmp, $targetFile, 85); // 85 is my choice, make it between 0 – 100 for output image quality with 100 being the most luxurious
}
}

//error_reporting(E_ALL);
$files=array();
$fdata=$_FILES['userfile'];
if(is_array($fdata['name'])){
for($i=0;$i<count($fdata['name']);++$i){
$files[]=array(
'name' =>$fdata['name'][$i],
'type' => $fdata['type'][$i],
'tmp_name'=>$fdata['tmp_name'][$i],
'error' => $fdata['error'][$i],
'size' => $fdata['size'][$i]
);
}
} else $errorMessage = "<p>Error uploading this file</p>";

//begin upload code
$continue = 1;
if (dirname(realpath("pics/" . $_GET["adr"] . "/")) == '/home/flyingfisch/public_html/shj/pics') {
$adr = "pics/" . $_GET["adr"] . "/";
}

else {
$dir = dirname(realpath('pics/' . $_GET['adr'] . '/'));
$errorMessage = $errorMessage . "<p>SECURITY ERROR: Dir error. If you have used the normal image upload form to get to this page, please contact the webmaster immediately.</p><p>DEBUG INFO: </p>";
$errorMessage = $errorMessage . "<p>DIRNAME REALPATH: " . $dir . "</p>";
$continue = 0;
}

$maxSize = 1000000000;

foreach ($files as $file) {
if(($file['type'] == "image/jpg") || ($file['type'] == "image/jpeg") || ($file['type'] == "image/pjpeg")) {
}
else {
$errorMessage = $errorMessage . "<p>Wrong File Type</p>
<p>Type: " . $file['type'] . "</p>
<p>Name: " . $file['name'] . "</p>
<p>" . var_export($file, true) . "</p>";
$continue = 0;
}

if($file['size'] > $maxSize) {
$errorMessage = $errorMessage . "<p>File is too large.</p>";
$continue = 0;
}

//check for errors
if(($file['error'] > 0)) {
echo "<p>Error" . $file['error'] . "</p>";
}

elseif($continue == 0) {
echo $errorMessage;
}

//display info
elseif($continue == 1) {
echo "Name:" . $file['name'] . "
";
echo "Type:" . $file['type'] . "
";

if((file_exists($adr . $file['name']))) {
echo "<p>" . $file['name'] . " already exists!</p>";
}
else {
//upload large version
move_uploaded_file($file['tmp_name'], $adr . $file['name']);

//resize and copy image for thumbnail
$work = new ImgResizer($adr . $file['name']);
$work -> resize(180, $adr . "/thumb/" . $file['name']);


echo "<p>Upload Successful! View file <a href='" . $adr . $file['name'] . "'>here.</a></p>
<p><a href='" . $adr . "/thumb/" . $file['name'] . "'>Thumbnail</a></p>";

}
}
}

?>


Does it look secure now?

#11 2072

2072

    Casio over god

  • Admin
  • PipPipPipPipPipPipPipPip
  • 1565 posts
  • Gender:Male
  • Location:Somewherebourg
  • Interests:Alternative states of consciousness, programming, making things work the best they possibly can.

  • Calculators:
    AFX2 ROM 1.02, CFX-9940GT+, FX-180P-Plus

Posted 10 July 2012 - 07:39 PM

Yep that's better but instead of solely relying upon realfilepath() to sort out the final directory, you should filter the value of $_GET["adr"] using a regular expression or replace it altogether using a dropdown input ('select') which would contain all possible values. If $_GET["adr"] is just a directory name, you can also use basename() to keep only the last component..

I would use a regular expression test such as if (preg_match('#^[^/.\\]+$#', $_GET['adr'])) { which would match a string containing anything but '/', '.' or '\'

#12 flyingfisch

flyingfisch

    Casio Maniac

  • Deputy
  • PipPipPipPipPipPipPipPip
  • 1891 posts
  • Gender:Male
  • Location:OH,USA
  • Interests:Aviation, Skiing, Programming, Mountain Biking.

  • Calculators:
    fx-9860GII
    fx-CG10 PRIZM

Posted 10 July 2012 - 10:30 PM

Yep that's better but instead of solely relying upon realfilepath() to sort out the final directory, you should filter the value of $_GET["adr"] using a regular expression or replace it altogether using a dropdown input ('select') which would contain all possible values. If $_GET["adr"] is just a directory name, you can also use basename()to keep only the last component..

I would use a regular expression test such as if (preg_match('#^[^/.\\]+$#', $_GET['adr'])) { which would match a string containing anything but '/', '.' or '\'


Ok. I think I'll look up preg_match(). I have heard of it but have not seen it used, however, it looks pretty useful.



Also tagged with one or more of these keywords: php, web design, upload, php image resize

1 user(s) are reading this topic

0 members, 1 guests, 0 anonymous users