I'm working on this snippet of code within a PHP file. The upload function is working but I suspect it is not secure. I move all the files into an upload folder within the site folder. I think that some of this could be tricked into uploading malicious files but I'm not sure how it would be done and more importantly, how to adjust this code to be more secure.

Basically what I'm trying to do is:

1. Allow Word and PDF documents to be uploaded to a sitefolder's upload folder.
2. Append the date to the filename.
3. Move the file from the PHP temporary directory into the destination direction mentioned in step 1.

Other note: I've read on some similar threads that files should not be uploaded to the wwwroot folder, but I'm not sure why that is the case. I understand that if a hacker gains access to some folder X and Y resides within X, that the hacker will have access to X and Y, but would the hacker necessarily be able to gain access to files up the www root directories?

<?php

$allowedExts = array("docx", "DOCX", "DOC", "doc","pdf","PDF");$temp = explode(".", $_FILES["file".$x]["name"]);
$extension = end($temp);
$destinationdirectory= "C:\inetpub\wwwroot\sitefolder\upload\\";$date=date("m-d-Y-his", time());
$originalname=(string)$_FILES["file".$x]["name"];$newfilename=$date." ".$originalname;
$newfilename=preg_replace('/\s+/', '',$newfilename);//Need to remove all white space to make url friendly
//Validation of files types
if ((($_FILES["file".$x]["type"] == "application/pdf") || ($_FILES["file".$x]["type"] == "application/msword")) && in_array($extension,$allowedExts)) {
if ($_FILES["file".$x]["error"] > 0) {
echo "Return Code: " . $_FILES["file".$x]["error"] . "<br>";
} else {
if (file_exists($destinationdirectory .$_FILES["file".$x]["name"])) { echo$_FILES["file".$x]["name"] . " already exists. "; } else { move_uploaded_file($_FILES["file".$x]["tmp_name"], destinationdirectory .$newfilename);
//Now that the file is validated, renamed and uploaded to the server, get the vars together to store them inside of the array.
$filelocation="http://mywebsite.com/upload/".$newfilename;
}//File moving success loop.
}
} //IF FILETYPE IS CORRECT AND ALLOWED.
else {
• write like this: $newfilename =$date . " " . $originalname; ### Simplify Other simplifications are possible. Instead of listing up all the lowercase and uppercase variants of extensions like this: $allowedExts = array("docx", "DOCX", "DOC", "doc","pdf","PDF");


You could use list just in lowercase, and when you need to check the extension, just wrap that in strtolower, like this:

in_array(strtolower($extension),$allowedExts)


You could reduce one nesting level in the conditions. Instead of this:

  if ($_FILES["file".$x]["error"] > 0) {
// ...
}
else {
if (file_exists($destinationdirectory .$_FILES["file".$x]["name"])) { // ... } else { // ... } }  You could use an elseif:  if ($_FILES["file".$x]["error"] > 0) { // ... } elseif (file_exists($destinationdirectory . $_FILES["file".$x]["name"])) {
// ...
} else {
// ...
}