Freescout: [Security] Unauthorised access to attachments

[TrueWinter]

While testing Freescout (I am currently using version 1.2.6), I have found that the email attachments can be accessed without needing to be logged in by accessing the URL.

For example:
An email attachment with the name image001.png will be uploaded to the server. This uploaded file should only be accessible while a user is logged in however, even after logging out, I can still access the image through the URL (Example: https://REDACTED/storage/attachment/5/2/1/image001.png)

This is a security risk as some businesses deal with confidential information which should only be accessible to staff and not unauthorised users on the internet.

[frommMoritz]

You could use a randomly generated UUID. For example: https://mail.example.com/storage/attachment/8/5/856b6674-4987-4794-8d8a-6f7d8c7d7af8/filename.extension. This Approach would preserve the original filename but also prevent the guessing of common filenames such as document.pdf, image001.png or logo.jpeg. I think it should be an option, if a user must be actually logged in to view the attachment or if the specific path is enough. In our use case (mostly as a great multi-user mail-system) it is helpful having the ability to access documents on the mobile where not everyone is logged in.

But I see that it, depending on the context, it is not advisable to allow unauthorized access.

This libary could be used: https://github.com/ramsey/uuid/

[Qrious]

We've used a different solution that is relatively simple to implement. Thought i'd share it, in case anyone is interested. If @freescout-helpdesk accepts this method, i'm willing to make a PR on the docs. We use the nginx auth-request module (requires apt-get install nginx-full on ubuntu) to authenticate the download request. Basically this module allows you to make an
auth-request for a location in nginx.

The ngx_http_auth_request_module module (1.5.4+) implements client authorization based on the result of a subrequest. If the subrequest returns a 2xx response code, the access is allowed. If it returns 401 or 403, the access is denied with the corresponding error code. Any other response code returned by the subrequest is considered an error.

This has as added advantage that the file to be downloaded does not have to pass through PHP, but instead can be delivered by Nginx like any other regular file (less memory usage, and buffer copying overhead).

Example configuration:

server {
   # Other config removed for brevity
    error_page 401 = @error401;

    location @error401 {
        return 302 https://example.com/login;
    }

    location ~ ^/storage/attachment/(.*)$ {
        try_files $uri $uri/;
        # Prevent cache servers from caching it
        add_header 'Cache-Control' "private";
        expires off;
        # This line is where the magic happens
        auth_request /auth;
    }

    location /auth {
        internal;
        # Do not pass the body, since that might contain parameters that interfere with the auth request
        proxy_pass_request_body off;
        proxy_set_header        Content-Length "";
        # Do send the cookies to laravel
        proxy_set_header                Cookie $http_cookie;
        proxy_pass https://example.com/authAttachment/check/;
    }

And our custom controller in a simple custom module (for now):

<?php

namespace Modules\AuthAttachment\Http\Controllers;

use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Routing\Controller;
use Illuminate\Support\Facades\Auth;

class AuthAttachmentController extends Controller {

   public function check(Request $request) {
        if (Auth::check()) {
            return response('Authenticated', 200);
        }
        return response('Unauthorized', 401);
    }
}

[freescout-helpdesk]

Attachments have to be publicly accessible. If we make attachments non-publicly accessible, your customers will be unable to download your attachments or embedded images.

[TrueWinter]

If the attachments have to be publicly accessible, can I suggest the following:

A unique token in a query string that is required to view files.

This solution would:

• prevent guessing of filenames as an attacker would also need the token of the file in order to download it
• keep the current file structure on the server (no UUID in filename as suggested above: https://github.com/freescout-helpdesk/freescout/issues/308#issuecomment-532345671)

So the file URL would be, for example, https://mail.example.com/storage/attachment/5/2/1/image001.png?token=344aab9758bb0d018b93739e7893fb3a

In order to not break access to old uploads, those could still be accessible without a token. For new uploads, add a random token to the database and match the token in the URL with the one in the database. I could always go into the database and manually add tokens for old uploads if required.

[freescout-helpdesk]

We've used a different solution that is relatively simple to implement. Thought i'd share it, in case anyone is interested. If @freescout-helpdesk accepts this method, i'm willing to make a PR on the docs.

More preferable way would be to use nginx X-Accel-Redirect header as it does not require nginx-full and it will let visitors to download the file instead of opening it in the browser.

location ~ ^/storage/attachment/(.*)$ {
    rewrite ^/storage/attachment/(.*) /index.php/attachment/download/?path=$1&token=$http_token last;
}
location /attachment/download/ {
    alias "${root}/storage/attachment/";
    add_header Content-type application/octet-stream;
    internal;
}

downloadAttachment() function in the PublicController.php:

public function downloadAttachment(Request $request) {
    $path = $request->path;
    $token = $request->token;

    // Get attachment by path and file name.
    $attachment = ...

    if (!$attachment) {
        abort(404, 'Attachment not found');
    }

    if (!Auth::check()) {
    // Check token.
    if (!Hash::check($attachment->id.$attachment->file_dir.$attachment->file_name, $token)) {
        // Redirect to the log in page.
    }
    }

    // Redirect user to internal location
    $internal_redirect = '/attachment/download/'.$path;
    // Create response
    // Set X-Accel-Redirect header to $internal_redirect
    // Return response
}

To download an attachment one need to be either logged or correct token should be provided in the attachment URL - https://github.com/freescout-helpdesk/freescout/issues/308#issuecomment-583318854. No need to store the token for each attachment in the DB, token can be generated and checked for each attachment:

$token = Hash::make($attachment->id.$attachment->file_dir.$attachment->file_name);

$is_valid = Hash::check($attachment->id.$attachment->file_dir.$attachment->file_name, $token);

[TrueWinter]

While that would be a better solution, would it not break the attachments for emails already sent as those would now require a token to be viewed? This was the main factor in my suggestion to use a database as old attachments could have a blank token in the database and would not need a token to be verified, where any new attachments would have a token added to the database.

I haven't really looked too much into the code for how attachments are handled, so my apologies if I'm understanding this wrong. If the proposed solution could work without affecting emails that have already been sent, then that would be great.

[freescout-helpdesk]

public boolean field (default: false) can be added to the attachments table to allow access to the old attachments. It can be set to true in the migration for records existing in attachments table.

[Qrious]

Did you already start working on it? Otherwise i'm willing to implement this. I think that it would be cleaner to add the token in the database and allow it to be null for backwards-compatibility.

if (!Auth::check()) {
    // Check token.
    if (!Hash::check($attachment->id.$attachment->file_dir.$attachment->file_name, $token)) {
        // Redirect to the log in page.
    }
    }

I believe we still have to check the token, even if the user is logged in, to prevent (authenticated) users from downloading attachments from mailboxes that they do not have access to? Do you agree?

[freescout-helpdesk]

Feel free to implement it.

Yes. Check token even if the user is logged in.

No need to store the token in the DB.

[Qrious]

I initially started to implement the solution you described. However this requires us to obtain the file hash on every request in order to have a secure token (if someone has access to the hash of the file, this implies they have had access to the contents of the file, making it a sufficient security token).

The hash you described above above does not suffice, since this data can all be obtained from the URL, so a malicious actor could gain access to the contents of the file by creating a valid token from the URL of the attachment.

Additionally, the Flysystem Storage API does not provide a 'quick' method to obtain the hash of a file. We would need to (potentially, depending on the driver) download/stream the file and hash it. This might easily result in a DOS vulnerability, since a client can easily send many simple requests that would require the server to download a large file from remote storage (Amazon S3 for example) every time to create a hash.

Concluding, I thought it is more efficient to store a (random) secret token on upload of the attachment in the database, since we already need to perform a database query to check if the file exists/if its token needs to be checked.

Finally I've made this change a bit more backwards compatible by not requiring X-Accel-Redirect for now. I'm thinking of adding an config option to enable X-Accel-Redirect headers for attachment download

[freescout-helpdesk]

The hash you described above above does not suffice, since this data can all be obtained from the URL, so a malicious actor could gain access to the contents of the file by creating a valid token from the URL of the attachment.

It's impossible to generate a hash from URL, as Hash::make function uses APP_KEY as a salt. Please use hashing algorithm described above.

[Qrious]

Unfortunately, this is not the current implementation. I've just confirmed this using the source code of the Hash function of Laravel 5.5 and tested it (see below). We can do either of two things:

• Use a random string per attachment as stored in the database.
• Prepend the APP_KEY ourselves. However, this would break all download links if we ever need to rotate the APP_KEY. This has already been recommended once by the Laravel core team. Secondly this also becomes a problem if the APP_KEY is lost for some reason. Finally it provides malicious actors with an oracle to guess our APP_KEY, since they can simply bruteforce our APP_KEY by attempting to download a file with a known id/file_dir and file_name (and varying the APP_KEY). If the download fails, the APP_KEY is incorrect. On a succesful download, they have retrieved our APP_KEY, and are able to decrypt/forge our cookies.

So i'd advise against using the second option.

The make function is implemented as:

public function make($value, array $options = [])
    {
        $hash = password_hash($value, PASSWORD_BCRYPT, [
            'cost' => $this->cost($options),
        ]);

        if ($hash === false) {
            throw new RuntimeException('Bcrypt hashing not supported.');
        }

        return $hash;
    }

I've tested it using the following inputs with this test script:

$attachment = new \stdClass();
$attachment->id = 2294;
$attachment->file_dir = "2/2/1/";
$attachment->file_name = "rule-1752412_1280-(1).jpg";

$hash = password_hash("22942/2/1/rule-1752412_1280-(1).jpg", PASSWORD_BCRYPT, [
            'cost' => 10
        ]);
var_dump(Hash::check($attachment->id.$attachment->file_dir.$attachment->file_name, $hash)); // This dumps 'bool(true)'

[freescout-helpdesk]

Thanks. We will polish the solution and make it compatible with Nginx's X-Accel-Redirect and Apache's X-Sendfile. It will take some time.

[freescout-helpdesk]

Implementation is in the master branch. Those who wish to test it, just pull from the master and run php artisan freescout:after-app-update. After that make sure that /storage/app/public/attachment folder has ben moved to /storage/app/attachment and if not - move it manually.

Now by default attachments are downloaded via PHP.

To enable downloading attachments via Apache's mod_xsendfile, enable it in the Apache and set APP_DOWNLOAD_ATTACHMENTS_VIA=apache in the .env file.

To enable downloading attachments via Nginx, set APP_DOWNLOAD_ATTACHMENTS_VIA=nginx in the .env file and add the following to the .conf file after location ~ \.php$ location:

location ^~ /storage/app/attachment/ {
    internal;
    alias /var/www/html/storage/app/attachment/;
}