I'm developing a system where the user will be able to upload a ".docx" file. Is verifying it's extension enough to know that this ".docx" file isn't infected?

Here's my upload PHP code:

<?php
session_start();

include("connection.php");
include("functions.php");

// Just to validate the user
$user_data = check_login($con);

include("connectionPostsDB.php");

if (isset($_POST['submit'])){

    $title = $_POST['title'];
    $tag = $_POST['tag'];
    $description = $_POST['description'];

    $file = $_FILES['file'];

    $fileName = $_FILES['file']['name'];
    $fileTmpName = $_FILES['file']['tmp_name'];
    $fileSize = $_FILES['file']['size'];
    $fileError = $_FILES['file']['error'];
    $fileType = $_FILES['file']['type'];

    $fileExt = explode('.', $fileName);
    $fileActualExt = strtolower(end($fileExt)); //here I get the actual file's extension (I hope xD)

    $allowed = array('docx');

    if(in_array($fileActualExt, $allowed)){
        if($fileError === 0){
            if($fileSize < 1000000){
                $fileNameNew = uniqid('', true).".".$fileActualExt;
                $fileDestination = '../imgs/posts/'.$fileNameNew;
                move_uploaded_file($fileTmpName, $fileDestination);

                $query = "INSERT INTO posts (title, descr, imgname, tag) 
                          VALUES ('".$title."','".$description."','".$fileNameNew."','".$tag."')";

                mysqli_query($postcon, $query);

                echo 'File successfully uploaded';
            }
            else {
                echo 'Your file is too big.';
            }
        }
        else {
            echo 'There was an error uploading your file.';
        }
    }
    else {
        echo 'This type of file not allowed.';
    }

}

So, me checking for the file's extension is enough to prevent some user to put some php code in my server (or do something to get information from the server)?

CodePudding user response：

No, just checking the extension isn't enough, however the main question is what you exactly mean by safe.

The code you've shown only stores the file on your server (under a server-generated name), and then publishes its existence in the database. Neither of those things themselves would be dangerous, even if the file contained the digital equivalent of the black plague.

But let's first take a step back; your SQL query is very much unsafe, though that has nothing to do with the uploaded files. You should read up on prepared statements as your entire site is wide open to SQL injection when you create queries this way.

Now, back on topic:

There are at least 2 things you could explore in order to become more relaxed about the .docx file being what it should be.

1. As far as i know, the docx format is really just a .zip file with some .xml files inside. PHP understands both those formats, so you could attempt to unzip it's contents, see if it contains the sorts of files you expect, including xml(s), and then see if those XML files are parse-able.

2. You could feed the uploaded file to a virusscanner

Neither of these methods is full proof, but they will both illuminate the situation a bit, allowing you (more specifically, your algorithm) to make a more informed decision.

Now as for what i ment with "what you exactly mean by safe". The actions you take here (merely renaming and storing the file and writing text to a database) are fundamentally pretty safe.

But i imagine there is someone or something else, on the other side of that database, that reads the same DB table, and allows someone to either open and or download that file. Now that action does actually have a lot of security implications.

Now if that database is only viewed through your application, then you could theoretically waive all responsibility and say "this file was anonymously uploaded by god knows who, treat it as if it arrived in your emails junk folder by unknown sender". But if you're taking any level of responsibility (be it implied or literal), you'll have to verify it to whatever degree you're comfortable with.

CodePudding user response：

Nope, it's never enough just chekcing the file extension because we can set .docx extension to any file type.

We should also check many more things like:

1. Is the mime type of the file expected?
2. Is the file size reasonable enough. For example, if someone upload a 25 GB docx file, then even if it's a valid docx file your server might go down.
3. Ideally you shouldn't upload any file in any production server instead you should upload in any storage provider like s3 bucket of AWS. So that even if that file is executable it's not being executed in your server. It's uploaded into a different server that is well optimized to handle files.

And many more such things to consider.