Basic File Upload issue

[afridy]

hi, i use the following code to upload a 13k jpg image to the server.
the script works successfully. problem is, uploaded file is not there in the server's 'attachment' folder.

Note:- The attachment folder is located at /public_html/attachment

<?php
if (($_FILES["file"]["type"] == "image/gif") || ($_FILES["file"]["type"] == "image/pjpeg")
   && ($_FILES["file"]["size"] < 20000)) {
 
  if ($_FILES["file"]["error"] > 0) {
    echo "Return Code: " . $_FILES["file"]["error"] . "<br />";
  }else{
    echo "Upload: " . $_FILES["file"]["name"] . "<br />";
    echo "Type: " . $_FILES["file"]["type"] . "<br />";
    echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
    echo "Temp file: " . $_FILES["file"]["tmp_name"] . "<br />";  
    
    /*if (file_exists("upload/" . $_FILES["file"]["name"])) {
       echo $_FILES["file"]["name"] . " already exists. ";
    }else{ */
    
       move_uploaded_file($_FILES["file"]["tmp_name"]. $_FILES["file"]["name"], "attachments/" . $_FILES["file"]["name"]);
       echo "Stored in: " . "attachments/" . $_FILES["file"]["name"];
    //}         
    }
}else{
echo "Invalid file";
}
?>

[Servyces]

Note:- The attachment folder is located at /public_html/attachment

move_uploaded_file($_FILES["file"]["tmp_name"]. $_FILES["file"]["name"], "attachments/" . $_FILES["file"]["name"]); 

I see a conflict

[afridy]

Hai, servyces, Thanks you for ther reply.
I put this code.

move_uploaded_file($_FILES["file"]["tmp_name"]. $_FILES["file"]["name"], "public_html/attachments/" . $_FILES["file"]["name"]); 

but no success. the file is not found in the attachments folder.
* i use filezilla for ftp.

[FSan]

Did you get what Servyces meant afridy?

If

The attachment folder is located at /public_html/attachment

then

"public_html/attachments/" will not work. See the extra "s"...?

I didn't thoroughly check your code though...
Was that just it?
Good luck

[kromey]

$_FILES["file"]["tmp_name"]. $_FILES["file"]["name"]
This is most decidedly not where you uploaded file is. Had you been checking the return code from move_uploaded_file (a good thing to do when something isn't working I would think...), you would find it returning FALSE. Had you turned on error reporting (also a good thing to do when something isn't working...), you would see an error to the effect of:
"Unable to open stream: cannot find file /tmp/blah.tmpmyuploadedfile.jpg"
(Where $_FILES["file"]["tmp_name"] is "/tmp/blah.tmp" and $_FILES["file"]["name"] is "myuploadedfile.jpg")

$_FILES['file']['tmp_name'] contains the full path to the temporary file - you do not need to append anything to get to your file. In fact, appending anything will cause you to fail to locate your file in 100% of cases.

Also, very bad to trust the user's supplied filename. What if I upload the file "../../etc/passwd"? Suddenly your script has just overwritten your server's passwd file with my file, and done it quite happily, too! At worst, you must validate the supplied filename to prevent such occurrences; at best, you should ignore the user's supplied filename and generate your own.

[afridy]

Did you get what Servyces meant afridy?

If

The attachment folder is located at /public_html/attachment

then

"public_html/attachments/" will not work. See the extra "s"...?

I didn't thoroughly check your code though...
Was that just it?
Good luck

Fscan, sorry. that was a typing mistake i put here in sitepoint. the folder name is really 'attachments'

Thank you for pointing out.

[afridy]

$_FILES["file"]["tmp_name"]. $_FILES["file"]["name"]
This is most decidedly not where you uploaded file is. Had you been checking the return code from move_uploaded_file (a good thing to do when something isn't working I would think...), you would find it returning FALSE. Had you turned on error reporting (also a good thing to do when something isn't working...), you would see an error to the effect of:
"Unable to open stream: cannot find file /tmp/blah.tmpmyuploadedfile.jpg"
(Where $_FILES["file"]["tmp_name"] is "/tmp/blah.tmp" and $_FILES["file"]["name"] is "myuploadedfile.jpg")

$_FILES['file']['tmp_name'] contains the full path to the temporary file - you do not need to append anything to get to your file. In fact, appending anything will cause you to fail to locate your file in 100% of cases.

Also, very bad to trust the user's supplied filename. What if I upload the file "../../etc/passwd"? Suddenly your script has just overwritten your server's passwd file with my file, and done it quite happily, too! At worst, you must validate the supplied filename to prevent such occurrences; at best, you should ignore the user's supplied filename and generate your own.

Thank you Krome for your Very Very valuable comments.
Let me try thouse suggessions

[afridy]

$_FILES["file"]["tmp_name"]. $_FILES["file"]["name"]
This is most decidedly not where you uploaded file is. Had you been checking the return code from move_uploaded_file (a good thing to do when something isn't working I would think...), you would find it returning FALSE. Had you turned on error reporting (also a good thing to do when something isn't working...), you would see an error to the effect of:
"Unable to open stream: cannot find file /tmp/blah.tmpmyuploadedfile.jpg"
(Where $_FILES["file"]["tmp_name"] is "/tmp/blah.tmp" and $_FILES["file"]["name"] is "myuploadedfile.jpg")

$_FILES['file']['tmp_name'] contains the full path to the temporary file - you do not need to append anything to get to your file. In fact, appending anything will cause you to fail to locate your file in 100% of cases.

Also, very bad to trust the user's supplied filename. What if I upload the file "../../etc/passwd"? Suddenly your script has just overwritten your server's passwd file with my file, and done it quite happily, too! At worst, you must validate the supplied filename to prevent such occurrences; at best, you should ignore the user's supplied filename and generate your own.

Kromey, Yes, you got the point. Working !!!

as you sad

$_FILES['file']['tmp_name'] contains the full path to the temporary file - you do not need to append anything to get to your file. In fact, appending anything will cause you to fail to locate your file in 100% of cases.

This is what make the code fails.

also the path should be "../attachments/" and not "public_html....

Follwoing is my new code.

<?php
if (($_FILES["file"]["type"] == "image/gif") || ($_FILES["file"]["type"] == "image/pjpeg")
   && ($_FILES["file"]["size"] < 20000)) {
 
  if ($_FILES["file"]["error"] > 0) {
    echo "Return Code: " . $_FILES["file"]["error"] . "<br />";
  }else{
    echo "Upload: " . $_FILES["file"]["name"] . "<br />";
    echo "Type: " . $_FILES["file"]["type"] . "<br />";
    echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
    echo "Temp file: " . $_FILES["file"]["tmp_name"] . "<br />";  
    
    /*if (file_exists("upload/" . $_FILES["file"]["name"])) {
       echo $_FILES["file"]["name"] . " already exists. ";
    }else{ */
    
       if  (move_uploaded_file($_FILES["file"]["tmp_name"], "../attachments/" . $_FILES["file"]["name"])) {
           echo "Successfully Moved";
           }else{
           echo "Unable to move the file";
       }
       //echo "Stored in: " . "public_html/attachments/" . $_FILES["file"]["name"];
    //}         
    }
}else{
echo "Invalid file";
}
?>

Thanks Every body for helping me.

[kromey]

I'm still going to stubbornly persist that you should not use $_FILES["file"]["name"] as the filename for the new file, at least not without some intensive validation. As-is, you are allowing any little script kiddie the ability to overwrite nearly any file on your server with whatever they want. Maybe it's just me, but I don't think this is A Good Thing.

Also, you're validating the file based upon the user-supplied file type ($_FILES["file"]["type"] comes from the browser and is not in any way verified by PHP or anything else on the server). This greatly exacerbates the security hole you've created by allowing me to choose the filename my file will be stored as - not only can I choose the filename and file location, but I can literally upload anything I want. Some of the things this makes possible include overwriting Apache's httpd.conf; replacing your system's /etc/passwd file, locking you out of your own server while allowing the script kiddie full access; replacing otherwise innocuous files (e.g. /bin/ls) with arbitrary binaries which would be executed with your permissions and not the (hopefully) limited permissions of Apache.

The fix is simple: replace the check against $_FILES["file"]["type"] with one against the file's extension (this will serve double-duty in forbidding things like virus.exe and slightly mitigating the flaw in using $_FILES["file"]["name"]). Once that passes, add another check using getimagesize, verifying that the image type is what you expect (and that the file is indeed an image!).

Then, do not use $_FILES["file"]["name"] to store the file, at least not without validating it first. See realpath, pathinfo, dirname, and basename to get you started. Best would be to generate a new filename, that way you know beyond a shadow of a doubt that you're not allowing any script kiddie to overwrite any of your files.

[afridy]

kromey, this is owsome tips. ok. i have few issues.

as in the script i upload the file to "../attachments" folder. so how can one could be put the file outside the "../attachments" unless he altred the php script which he could not? (coz he dont no the password)

[kromey]

It's very simple. Consider what would happen if the contents of $_FILES['file']['name'] are "../../etc/passwd". This means that your full path becomes "../attachments/../../etc/passwd"; if attachments is 2 directories deep in your file structure, then you've just let me overwrite /etc/passwd!

Or consider if the name I supply is "/etc/passwd". The path would become "../attachments//etc/passwd", which may be interpreted as "/etc/passwd" (this one's been inconsistent in my testing - not sure if it's kernel version or shell or what that affects it, but safest to assume you're at risk for it!).

Neither of these require doing anything to alter the PHP file - all it requires is manipulating the data that I'm sending to your server, data which you are treating as trustworthy and safe and which is in fact neither.

Literally everything in $_FILES (except $_FILES['file']['error'] and $_FILES['file']['tmp_name']) is supplied by the user. This means that someone who knows what they're doing, or some script kiddie with the right software, can provide anything they want. This puts the examples I gave above within unbelievably easy reach.

[afridy]

First of all sorry for the bit delayed as i had to consentrate on another issue.

Yes, Kromey, i undestand the real security issue. Thank you so much for pointing out this threat.

so you recon me to check the extensions of the files insted of $_FILES['file']['type']. ok . i will fix this matter now. i have to use an string manipulation function.

So i will fix this issue and come to the next point, probably a little later.

bye for now ..

[afridy]

Hai Kromi,

my native programming language is VB6. i am just 6 months old to php.
please help me to trim the extension part from a file.

i mean what function and how wil be the code like?

[afridy]

hai Kromi,

i think it should be rtrim(string,char). am i correct?

[kromey]

Take a look at pathinfo - it'll do all the work for you!

[afridy]

Yes kromey, it don the trick.

Off Topic:
How do i rate helpfull posts / or Give them reputaion in sitepoint?

[kromey]

Off Topic:

So far as I know, there is no reputation system here. Nor is there any post rating system.