Hacker used picture upload to get PHP code into my site

  • I'm working on a website — right now it's in early stages of testing, not yet launched and just has test data - thank goodness.

    First of all, a hacker figured out the password to log onto the websites 'administration' pages*. I think they used a key logger on a friend's computer who logged into the site to give me feedback.

    Secondly, they used a picture upload box to upload a PHP file. I have put in strict checking so that only .jpg and .png files are accepted — everything else should have been rejected. Surely there is no way to upload a .jpg file and then change the extension once the file is stored?

    Thankfully I also generate new file names when a file is sent to me, so I don't think they were able to locate the file to execute the code.

    I just can't seem to figure out how the website let a PHP file through. What's wrong with my security? The validation function code is below:

    function ValidateChange_Logo(theForm)
       var regexp;
       if (theForm.FileUpload1.value == "")
          alert("You have not chosen a new logo file, or your file is not supported.");
          return false;
       var extension = theForm.FileUpload1.value.substr(theForm.FileUpload1.value.lastIndexOf('.'));
       if ((extension.toLowerCase() != ".jpg") &&
           (extension.toLowerCase() != ".png"))
          alert("You have not chosen a new logo file, or your file is not supported.");
          return false;
       return true;

    Once the file gets to the server, I use the following code to retain the extension, and generate a new random name. It is a bit messy, but it works well.

    // Process and Retain File Extension
    $fileExt = $_FILES[logo][name];
    $reversed = strrev($fileExt);
    $extension0 = substr($reversed, 0, 1);
    $extension1 = substr($reversed, 1, 1);
    $extension2 = substr($reversed, 2, 1);
    $fileExtension = ".".$extension2.$extension1.$extension0;
    $newName = rand(1000000, 9999999) . $fileExtension;

    I've just tested with a name such as logo.php;.jpg and although the picture cannot be opened by the website, it correctly changed the name to 123456.jpg. As for logo.php/.jpg, Windows doesn't allow such a file name.

    * Protected pages on the website that allow simple functions: like uploading a picture that then becomes a new logo for the website. FTP details are completely different to the password used to log onto the protected pages on the website. As are database and cPanel credentials. I've ensured that people can't even view the folder and file structure of the site. There is literally no way I can think of to rename a .jpg, or .png extension to .php on this site if you don't have FTP details.

    Comments are not for extended discussion; this conversation has been moved to chat.

    Short answer: client side validation can easily be bypassed by setting a proxy. They can just submit a.jpg first and pass all the checkings. The request will then intercepted at the proxy and changed to a.php. You need to do validation on server side.

  • Anders

    Anders Correct answer

    4 years ago

    Client side validation

    The validation code you have provided is in JavaScript. That suggests it is code that you use to do the validation on the client.

    Rule number one of securing webapps is to never trust the client. The client is under the full control of the user - or in this case, the attacker. You can not be sure that any code you send to the client is used for anything, and no blocks you put in place on the client has any security value what so ever. Validation on the client is just for providing a smooth user experience, not to actually enforce any security relevant constraints.

    An attacker could just change that piece of JavaScript in their browser, or turn scripts off completely, or just not send the file from a browser but instead craft their own POST request with a tool like curl.

    You need to revalidate everything on the server. That means that your PHP must check that the files are of the right type, something your current code doesn't.

    How to do that is a broad issue that I think is outside the scope of your question, but this question are good places to start reading. You might want to take a look at your .htaccess files as well.

    Getting the extension

    Not a security issue maybe, but this is a better way to do it:

    $ext = pathinfo($filename, PATHINFO_EXTENSION);

    Magic PHP functions

    When I store data from edit boxes, I use all three of PHP's functions to clean it:

    $cleanedName = strip_tags($_POST[name]); // Remove HTML tags
    $cleanedName = htmlspecialchars($cleanedName); // Allow special chars, but store them safely. 
    $cleanedName = mysqli_real_escape_string($connectionName, $cleanedName);

    This is not a good strategy. The fact that you mention this in the context of validating file extensions makes me worried that you are just throwing a couple of security related functions at your input hoping it will solve the problem.

    For instance, you should be using prepared statements and not escaping to protect against SQLi. Escaping of HTML tags and special characters to prevent XSS needs to be done after the data is retrieved from the database, and how to do it depends on where you insert that data. And so on, and so on.


    I am not saying this to be mean, but you seem to be doing a lot of mistakes here. I would not be surprised if there are other vulnerabilities. If your app handles any sensitive information I would highly recommend that you let someone with security experience have a look at it before you take it into production.

    To take a simple example: Open your website in Chrome. Press f12 to open the developer tools. Select the sources tab. You'll see all your JavaScripts, lined up and nicely editable. Find your validation function. Delete it, and replace with return true;

    /\ Not only in Chrome

    Comments are not for extended discussion; this conversation has been moved to chat.

    One thing that important to emphasize is "**re**validate everything" -- just because you are validating on the server doesn't mean you shouldn't in the browser. Just don't use it for *security*. This way your users can know things like "oh I put an A in my phone number, whoops" before sending the form across.

    Right, validation on the user's side should just be used to decrease the amount of requests and as such increase speed.

    And to make things even more interesting, there's always polyglots... http://blog.portswigger.net/2016/12/bypassing-csp-using-polyglot-jpegs.html

    Using javascript only validation mean no validation at all. it allow to post via curl

    @Cryptopat something something Node.js

    `I think they used a key logger on a friend's computer` well no, this like happen really rarely because lots of sites are full of vulnerabilities easier to exploit.

    I wouldn't check a file type looking at the file extension. There are tools out there to check the content of a file and let you know what type of file it is (`file` in linux is an example).

    @YoMismo Since the extension determines what is executed and not by the server, I would definately check it. But your suggestion might be a good addition to that.

    @Anders, there may be restrictions to upload some kind of files (executables) but not others (images), if you only check file extensions instead of the contents someone may be able to upload an executable. That may lead to the execution of that file talking adventage of another vulnerabilities. And on other OS than Windows an executable has nothing to do with file extensions.

    @YoMismo With standard config the `.php` extension very much determines if a file is executed or not, be it on Linux or not. Not saying checking what is actually in the file is bad, just saying if you do not also check the extension you have a problem.

License under CC-BY-SA with attribution

Content dated before 7/24/2021 11:53 AM