# PHP upload script might be vulnerable

I have written a basic PHP upload script a few years ago and recently I discovered some phishing html pages in the website's root.

This website is located in /www/sitename, while I upload the files to /gstorage/sitename, which can't be accessed from the web.

I have found a lot of html files in /www/sitename and it seems like they somehow came through the upload.php file, as I've found a POST from a specific ip to this file in the logs before the files appeared and then that IP started sending post to those files.

Here's the code:

include('inc.php');
$disallowed = array('php', 'php5'); if(isset($_FILES['upl']) && $_FILES['upl']['error'] == 0){$extension = pathinfo($_FILES['upl']['name'], PATHINFO_EXTENSION); if(in_array(strtolower($extension), $disallowed)){ echo '{"status":"error", "message":"no PHP"}'; exit; }$url = findNewURL();
$size = formatSizeUnits($_FILES['upl']['size']);
if(move_uploaded_file($_FILES['upl']['tmp_name'], '/gstorage/edited/'.$url)){
try {
$stmt =$db->prepare("INSERT INTO files(url, filename, size, type) VALUES(:url,:filename,:size, :type)");
$stmt->execute(array(":url" =>$url, ":filename" => $_FILES['upl']['name'], ":size" =>$size, ":type" => $_FILES['upl']['type'])); } catch(PDOException$ex) {
echo 'Error!';
echo $ex->getMessage(); } echo '{"status":"success", "link": "http://edited.org/'.$url.'"}';
exit;
}
}
echo '{"status":"error"}';
exit;


Where could the vulnerability be?

• What does findNewURL() do? Jul 2 '16 at 14:12
• Here it is: function findNewURL() { global $db;$file = generateRandomString(5); try { $stmt =$db->query("SELECT url FROM files WHERE url='$file'");$row = $stmt->fetch(PDO::FETCH_ASSOC); } catch(PDOException$ex) { return 'error'; } if($row) findNewURL(); return$file; }
– Alex
Jul 2 '16 at 14:18
• Suggestion, move the findNewURL() code to the question. You can do this before someone attempts to answer. Jul 2 '16 at 14:40
• Looks like you’re missing a return before the recursive findNewURL().
– Ry-
Jul 2 '16 at 19:37

Your extension check is pretty much useless, as I can still upload a variety of files with extensions that can gain me code execution, such as php4 or .htaccess. If possible, use a whitelist approach instead.

But in your case, it doesn't really matter, as you are not using the extension anyways, the filename is just a random 5 character string without extension.

You should also always use prepared statements. Yes, \$file is currently controlled by you, but who knows, that may change in the future.

Apart from this, I don't see any vulnerabilities in your script. My guess is that the vulnerability is somewhere else (seeing the logged request that you suspect would still be interesting).

• I know the script is pretty bad as I've learned a lot since writing that, but I am curious to find the vulnerability.
– Alex
Jul 2 '16 at 15:40
• @Alex For a script that was written a couple of years ago by a beginner it is actually pretty good. You do use prepared statements, you remove the file extension, so there really are no real security issues here. You should look at the rest of your code for them. Just out of interest: What is the POST request that you suspect of exploiting this code?
– tim
Jul 2 '16 at 15:44
• I have hundreds of POST requests from an IP trying to find php scripts (admin.php, header.php, footer.php etc.) and this is the request before starting to access his uploaded scripts: 13:4303:187.192.129.231 - - [26/Jun/2016:07:40:55 +0200] "POST /upload.php HTTP/1.1" 200 91 "http://edited/" "Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.0.11490 Safari/537.36"
– Alex
Jul 2 '16 at 15:50
• The next POST is to a file he managed to upload: 49:81229:187.192.129.231 - - [27/Jun/2016:03:43:26 +0200] "POST /settings.php HTTP/1.1" 200 843 "http://edited/settings.php" "Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.0.11490 Safari/537.36" which is a file manager and a wrapper for console commands.
– Alex
Jul 2 '16 at 15:51
• @Alex Hm, that is odd, and it does somewhat suggest that this script is vulnerable (although the timestamps suggest that the requests are rather far apart). But unless I'm totally overlooking something, it shouldn't be. There really is no way for an attacker to upload a file called settings.php, as you set the filename of the uploaded file yourself. You may want to post the code that retrieves the files if such code exists, as the problem may be at that end.
– tim
Jul 2 '16 at 16:00