Is verifying a file's extension enough to know that the file isn't hiding a virus (or something that can infiltrate my server)?

Question

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)?

**Warning:** You are wide open to [SQL Injections](https://php.net/manual/en/security.database.sql-injection.php) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) — Dharman, Dec 26 '21 at 23:12

Raxi · Accepted Answer · 2021-12-26T23:06:37.760

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.

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.
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.

The sql part was just for testing. I'm actually gonna upload the files from a WPF app to the webserver, and then download the files in other computers that logs with that account. So basically all it's doing is storing and allowing the user (same who uploaded) to download it. Is my webserver in any risk doing this? — Soulss, Dec 26 '21 at 23:06
No it isn't, the worst it could do is fill up your servers disk space which would cause a DoS. You could refuse accepting more uploads if the disk space available is less than *some threshold* if you want to protect against that. If however the serveradmin (or someone else with access) downloads these files from the server directly, then again, everything is up in the air. — Raxi, Dec 26 '21 at 23:09
I see... I'm actually gonna limit the disk space each user can use (like upload to a max of 40mb of "docx" files). My only concern was if somehow someone could upload a file with "docx" that could potentially gain access to my DB and user passwords... But i'm only using it to sync the folders of the users. — Soulss, Dec 26 '21 at 23:12
Well, the way your query is structured, everyone in the world can already access your database :) You should really read up on prepared statements, the following url gives some decent info; also it is likely you've made the same mistake in all other queries in your application, they'll all need to be changed; https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php/60496#60496 — Raxi, Dec 26 '21 at 23:17

score 0 · Answer 2 · answered Dec 26 '21 at 22:50

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:

Is the mime type of the file expected?
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.
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.

Is verifying a file's extension enough to know that the file isn't hiding a virus (or something that can infiltrate my server)?

2 Answers2