[Jack Drake]

I am having an issue with moving a file that has been uploaded to a certain folder.

Code

move_uploaded_file(filename, destination)

The issue I'm getting is that it won't move the file to a specified folder.
I have also granted chmod to the folder to allow writing to it

Why isn't it working?

[MoreCreativeUsername]

Do you get an error message? Can you show us the exact code you are using to generate destination etc?

To enable error messages:

PHP Code


<?php 
ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL); 


[Nachfolger]

I was dealing with the same issue last night. Check your PHP error log, I 100% ensure you there is an error there. My issue was that only root had access to the file, obviously the webserver needs access too.

[Abruptum]

I am having an issue with moving a file that has been uploaded to a certain folder.

Code

move_uploaded_file(filename, destination)

The issue I'm getting is that it won't move the file to a specified folder.
I have also granted chmod to the folder to allow writing to it

Why isn't it working?

Have you checked the paths ? doest he file exists before you try to move it ?
Maybe you got a problem with special characters.

[Money-]

Just like the above user said. Might be a simple file path error here. I guess probably not.

[Charger]

Check your error log. 99% chance it’s a permission issue.

[Numinous]

Use the following code:

PHP Code


<?php 
     if(isset($_FILES["file"]["type"]))
          {
              $validextensions = array("jpeg", "jpg", "png", "gif");
              $temporary = explode(".", $_FILES["file"]["name"]);
              $file_extension = end($temporary);
              $rand = rand();
              $filename = $rand."_".$_FILES["file"]["name"];

              if (($_FILES["file"]["size"] < 3000000) && in_array($file_extension, $validextensions)) 
                {
                    if ($_FILES["file"]["error"] > 0)
                    {
                        echo "Return Code: " . $_FILES["file"]["error"] . "<br/><br/>"; 
                    }
                    else
                    {
                        if (file_exists("upload/" . $filename)) 
                        {
                            $error = "Error! File Already Exists."; 
                        }
                        else
                        {   
                            
                            $sourcePath = $_FILES['file']['tmp_name']; // Storing source path of the file in a variable
                            $targetPath = "upload/".$filename; // Target path where file is to be stored
                            move_uploaded_file($sourcePath,$targetPath) ; // Moving Uploaded file
                            
                            $filepath = htmlspecialchars($targetPath);
                                            
                            $sql299 = "UPDATE users set imagename =?, imagepath=?, lastupdateimage=? where username = ?";
                            $stmt299 = $conn->prepare($sql299); 
                            $stmt299->execute(array($filename,$filepath,$datetime,$user));
                            $success = "Success! File Uploaded Successfully!"; 
                        }
                    }
                }
              else
              {
                $error = "Error! Invalid file Size or Type";
              }
          } 


[realsunjester]

i would suggest.

https://www.php.net/manual/en/language.exceptions.php

[Nachfolger]

Use the following code:

PHP Code


<?php 
     if(isset($_FILES["file"]["type"]))
          {
              $validextensions = array("jpeg", "jpg", "png", "gif");
              $temporary = explode(".", $_FILES["file"]["name"]);
              $file_extension = end($temporary);
              $rand = rand();
              $filename = $rand."_".$_FILES["file"]["name"];

              if (($_FILES["file"]["size"] < 3000000) && in_array($file_extension, $validextensions)) 
                {
                    if ($_FILES["file"]["error"] > 0)
                    {
                        echo "Return Code: " . $_FILES["file"]["error"] . "<br/><br/>"; 
                    }
                    else
                    {
                        if (file_exists("upload/" . $filename)) 
                        {
                            $error = "Error! File Already Exists."; 
                        }
                        else
                        {   
                            
                            $sourcePath = $_FILES['file']['tmp_name']; // Storing source path of the file in a variable
                            $targetPath = "upload/".$filename; // Target path where file is to be stored
                            move_uploaded_file($sourcePath,$targetPath) ; // Moving Uploaded file
                            
                            $filepath = htmlspecialchars($targetPath);
                                            
                            $sql299 = "UPDATE users set imagename =?, imagepath=?, lastupdateimage=? where username = ?";
                            $stmt299 = $conn->prepare($sql299); 
                            $stmt299->execute(array($filename,$filepath,$datetime,$user));
                            $success = "Success! File Uploaded Successfully!"; 
                        }
                    }
                }
              else
              {
                $error = "Error! Invalid file Size or Type";
              }
          } 


I hate when people critique my code without providing examples of their own, but I'm going to do that here. I think it's awfully negligent to assume users are uploading legitimate files with correct extensions. I could just upload a PHP file renamed with the extension .png and this script would simply allow it. I would recommend any user considering using this code to go way beyond what you've provided here, and actually provide legitimate security measures when validating files. You also check the file extension before verifying the file was uploaded without errors. I hate getting errors in my error log, and this could definitely cause an error, particularly if the file was not uploaded successfully.

A few things I did when managing user uploads:
- Rename the file (sha1_file or md5_file)
- Validate ACTUAL mime against a whitelist
- Validate PROVIDED mime against a whitelist
- Validate provided mime and ACTUAL mime (If the actual mime is not what was provided, we could have a potential exploiter)
- Ensure the file provided was an UPLOADED file (Prevents someone from tricking PHP into thinking an existing file was uploaded)
- Move the file outside of the document root (Important)

These are things off of the top of my head that I included, and I would recommend everyone using this script to take the time to do their own research to ensure every aspect of the validation is covered.

Would love to hear feedback from anyone critiquing my critique.

[eithan1231]

EDIT: for some reason my quote didn't work... anyway, i was trying to quote @Nachfolger second post.

this is my first post on HF for a long while... anyway...

I do agree with you on most of your points, with the exception of the MIME stuff. MIME types are as easily spoofable as the file extension. I personally wouldn't use MIME types, I would just stick with the file extension. At-least for the file extension, it uploads what the user intends to upload. The only way to get some true assurance of the MIME type, is to manually validate it with some sort of header validation - but even this isn't fall-proof.

Now, I want to bash on that persons code as well.
- One directory for all uploads? really? cmon fam.
- Hard-coding the upload directory. (what bout if yall get a cdn or some shit? "nothin Varnish cant handle")
- Vulnerable to directory traversal attack, smart! (hashing it as Nachfloger said would fix this)
- Hard-coding limits (file size, valid extensions) really fam? atleast tell the user the limits....
- Assuming there is a dot in the filename. I love me some unforeseen errors or unexpected return results.
- You say "use this code", it's designed around one of your systems. It's not generic code that OP can use.
- Your error handling is big big meme. (and by big big meme, i mean it's barely existent)
- Inconsistent variable naming lets do a little hacker_styleWith_Inconsistencies. (I am guilty of this)
- Permission checks. (especially important for nix) see https://www.php.net/manual/en/function.fileperms.php
- Misuse of double quotations. Use singular quotes unless double is needed. (not urgent, but can bring performance improvements if you have thousands of strings)
- Assuming working directory. 'upload/{filename}'. You should make stuff like this an absolute directory.
- $sql200, $sql201, $sql202... really fam? unless they're global, and you ACCESS them using the global keyword, this aint needed....
- Type validation for sql input.
- Your SQL table uses username as the condition.... use an identifier... indexes for id's are smaller and therefore it can result in a quicker more efficient query.

tldr: code is garbo. if you use it in production, I feel sorry for your server.

[Nachfolger]

EDIT: for some reason my quote didn't work... anyway, i was trying to quote @Nachfolger second post.

this is my first post on HF for a long while... anyway...

I do agree with you on most of your points, with the exception of the MIME stuff. MIME types are as easily spoofable as the file extension. I personally wouldn't use MIME types, I would just stick with the file extension. At-least for the file extension, it uploads what the user intends to upload. The only way to get some true assurance of the MIME type, is to manually validate it with some sort of header validation - but even this isn't fall-proof.

I 100% agree that mime types are spoofable. In fact, anything user provided is spoofable. Period. I just want to make a distinction between what you interpreted vs what I meant. When I said ACTUAL mime type, I meant reading into the file header and making a highly educated guess as to what type of file it is. I used an existing and open source library from Github to read the header magic, and based on my testing proved to be very accurate.

Again, anything user provided is spoofable, including the file header, but it’s better than relying solely on the file extension if you ask me. I would prefer to have two validations over just one.

Otherwise, thanks for the feedback. I just wanted to clear that up.

[eithan1231]

I 100% agree that mime types are spoofable. In fact, anything user provided is spoofable. Period. I just want to make a distinction between what you interpreted vs what I meant. When I said ACTUAL mime type, I meant reading into the file header and making a highly educated guess as to what type of file it is. I used an existing and open source library from Github to read the header magic, and based on my testing proved to be very accurate.

Again, anything user provided is spoofable, including the file header, but it’s better than relying solely on the file extension if you ask me. I would prefer to have two validations over just one.

Otherwise, thanks for the feedback. I just wanted to clear that up.

Ah okay, thx for the clarification. Was not entirely sure what you meant by ACTUAL MIME type. Makes sense now. And yeah, I have to agree with you. Though I'm probably still going to continue using my simple extension check. But frankly, all the sites I've made have no file uploads, so this doesn't affect me in any way.

[Disease]

Ah okay, thx for the clarification. Was not entirely sure what you meant by ACTUAL MIME type. Makes sense now. And yeah, I have to agree with you. Though I'm probably still going to continue using my simple extension check. But frankly, all the sites I've made have no file uploads, so this doesn't affect me in any way.

If this upload is exposed to the public then I think you're making a mistake. At least upload the files outside of your document root and prevent them from being executed.

[eithan1231]

If this upload is exposed to the public then I think you're making a mistake.  At least upload the files outside of your document root and prevent them from being executed.

No you're not making a mistake by uploading to a publicly accessible folder. Only an idiot would manually handle static content. (no offence.) Let me explain why.

Let's say for example you're using Nginx. If you upload static content to a directory within Nginx's www directory, you allow Nginx to handle the content. This is factors more performant than manually getting PHP to handle the delivery. PHP is a power consuming pile of shit, and should be avoided whenever possible. PHP requires an entire thread to process a request (sometimes an entire process), whereas Nginx is event based and requires 1 thread for many requests. Event based > Thread based architectures. PHP also consumes a tonne (under exaggeration) of ram to process a request script, lets say 5mb with all overhead associated with running a PHP script. That is A LOT in comparison to the couple couple of kilobytes that nginx would use per static request. Not to mention all the additional parsing that PHP would need to do, not to mention the parsing of the PHP script itself.

There is exceptions to this, ie: you want permission checks for a file. or? idk. not many exceptions. facebook, discord, and many other big websites don't even have permission checks for viewing images sent in private messages (not even kidding).

But having said this, you should ensure it's safe to upload to your web server directly. (check extensions, mime types, nginx/apache configurations, etc)

Also when I say PHP is shit, I say it with love. PHP is <3, but PHP also has its flaws.

[Nachfolger]

No you're not making a mistake by uploading to a publicly accessible folder. Only an idiot would manually handle static content. (no offence.) Let me explain why.

Let's say for example you're using Nginx. If you upload static content to a directory within Nginx's www directory, you allow Nginx to handle the content. This is factors more performant than manually getting PHP to handle the delivery. PHP is a power consuming pile of shit, and should be avoided whenever possible. PHP requires an entire thread to process a request (sometimes an entire process), whereas Nginx is event based and requires 1 thread for many requests. Event based > Thread based architectures. PHP also consumes a tonne (under exaggeration) of ram to process a request script, lets say 5mb with all overhead associated with running a PHP script. That is A LOT in comparison to the couple couple of kilobytes that nginx would use per static request. Not to mention all the additional parsing that PHP would need to do, not to mention the parsing of the PHP script itself.

There is exceptions to this, ie: you want permission checks for a file. or? idk. not many exceptions. facebook, discord, and many other big websites don't even have permission checks for viewing images sent in private messages (not even kidding).

But having said this, you should ensure it's safe to upload to your web server directly. (check extensions, mime types, nginx/apache configurations, etc)

Also when I say PHP is shit, I say it with love. PHP is <3, but PHP also has its flaws.

I was trying to avoid coming back to this thread, but I feel as if your response yields an opposite opinion.

The topics you cover are pretty ignorant of obvious security aspects. I can't speak to your personal experiences, but within my own experience, it would be awfully negligent to not protect user uploads. Static content or not, users and their uploads deserve to be protected. I'm not sure anyone made the claim that using PHP to serve a file would be more efficient than the web server, but you seem to be basing your argument solely for the purpose of efficiency. The good news is, you won't need to worry about the extra overhead of serving files when all of your user's uploads get leaked and they all abandon whatever project you're referencing.

Acknowledging that this thread is in the PHP Subforum, no one truly made the claim that PHP is the only backend solution. If you think PHP consumes too many resources, there's always C++/Java/Node/Python/Lua. I'm sure you can find the time to benchmark them and pick the best solution over PHP.

As a side note, just because Facebook and Discord don't have permission validation before serving files, doesn't really mean that there aren't legitimate reasons or applications to do so. Besides, taking into account Facebook's recent history in terms of their privacy and security, are they truly the best example for this?