Directory traversal vulnerability (CVE-2022-40734)

[silicahd]

I do not use this package, but I seen this in our firewall blocked list. My guess is that there is some sort of vulnerability.

[johseg]

yes that is a security vulnerability. I found that during a pentest for a client and wanted to report this privately, but since the picture shows the attack the cat is out of the back anyway

POC with curl: curl "https://$DOMAIN/laravel-filemanager/download?working_dir=%2F../../../../../../../../../../../../../../../../../../../etc&type=Files&file=passwd"

This will give you the file /etc/passwd on the server. All files which are readable by the user used to run the application can be read. This usually includes configuration files with passwords etc.

I want to request a CVE for this for proper tracking. Any objections to this by the maintainers? I'll wait a week until I request one.

[duyld2108]

Just protected lfm routes with middleware [auth] until this issue fixed.

[johseg]

This was assigned CVE-2022-40734.

@silicahd: Can you please change the title to something like "Directory traversal vulnerability (CVE-2022-40734)"?

[jelmersdp]

@duyld2108 is it possible that this is already the case. If I look at the package code I see that the auth middleware is already called. Are there any other specific lfm route that need to be include in the auth middleware?

[Jacotheron]

Having spent quite a bunch of time to get familiar with this issue (after I noticed no further releases to fix this issue), I am quite confident that this issue can be marked as resolved, and the releases no longer being marked as insucure.

When using any of the local filesystem drivers (managed by League\Flysystem) will be inspected and get the path normalized (luague/flysystem/src/WhitespacePathNormalizer.php - WhitespacePathNormalizer.php). Inside this class, the method "normalizeRelativePath" will split the path up into its various sections (relative to the root for the laravel disk). This will then rebuild the path to never allow access to anything outside of this root. Should it find that the relative path wants to escape the root, it will throw a PathTraversalDetected exception. This existed for at least 3 years already

From my stacktrace, this happens when LFM determines whether the file exist, prior to fetching it to serve.

The way this is written is quite bulletproof, and other errors will be thrown if it detects possible ways one might try to fool it. I attempted this traversal on my own local server, throwing a bunch of the common directory traversal options (from Directory Traversal Cheat Sheet) at it, and it blocked most, and the others failed as they were so wacky, PHP could not decode them into a path (the file does not exist, thus error 404).

I actually tried to make a pull-request with a bit of basic changes to fix this issue, but was unable to test it to confirm that it works as expected. My strategy was to simply use the PHP "realpath" function, where a relative path can be entered and it normalizes it into an absolute path; then ensure that the laravel disk root is above the normalized real path. For anyone wanting the code:

DownloadController -> getDownload method contents

       $file = $this->lfm->setName(request('file'));
        $file_absolute = $file->path('absolute');

        if (!Storage::disk($this->helper->config('disk'))->exists($file->path('storage'))) {
            abort(404);
        }

        if(config('filesystems.disks.'.$this->helper->config('disk').'.driver') === 'local'){
            $disk_root = realpath(config('filesystems.disks.'.$this->helper->config('disk').'.root'));
            $file_real_path = realpath($file_absolute);
            if(!Str::startsWith($file_real_path, $disk_root)){
                abort(404);
            }
        }

        return response()->download($file_absolute);

I was unable to get any request to pass the first "abort(404)", while attempting to go outside of the disk root.

As a conclusion, while this might have been an issue in the past, it has been solved (yes, it might not stop bots attempting to break in). While I would like these types of issues to be solved within the package that exposes it, the fact that is fixed by the underlying abstraction layer is good enough for me.

Just an edit: my tests is run with Laravel v9.40.1 (latest at time of writing), and PHP 8.1

[WhereIsLucas]

So, should we consider this issue resolved and tag a new release ?

[shankhadevpadam]

Is this issue solved?

[Romkabouter]

bump

[streamtw]

Thank you @Jacotheron for your detailed elaboration! Looks like the issue will not exist if the version of league\flysystem is above 2.0.0.

I am finding a way to resolve the CVE.

[streamtw]

I have added dependency for league/flysystem in composer.json, and released v2.6.4 of this package. If anyone installed this package with v1.x of league/flysystem, there will be a warning in the output of composer once they update this package to v2.6.4.

[streamtw]

I have addressed the patched versionv2.6.4 of this CVE in the Github Advisory Database(https://github.com/advisories/GHSA-5m2h-7rf2-rpx6).

I will still try to update information on other CVE websites. But I think it is fine to mark this issue as solved.

[sw0rd1ight]

Does this vulnerability really exist? I can’t reproduce @johseg @streamtw my environment league/flysystem 1.1.10 laravel/framework 8.83.27 unisharp/laravel-filemanager:2.6.0

GET /laravel/html/laravel-filemanager/download?working_dir=%2F../../../../../../../../../../../../../../../../../../../etc&type=Files&file=passwd HTTP/1.1
Host: localhost:82
Cache-Control: max-age=0
sec-ch-ua: "Chromium";v="91", " Not;A Brand";v="99"
sec-ch-ua-mobile: ?0
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.114 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
Sec-Fetch-Site: none
Sec-Fetch-Mode: navigate
Sec-Fetch-User: ?1
Sec-Fetch-Dest: document
Accept-Encoding: gzip, deflate
Accept-Language: zh-CN,zh;q=0.9,en;q=0.8
Cookie: session=000573dc-ed50-4fc7-8243-5cb81b5c0b81.OzwBLib5YzCDnEDtAlSU7vizqY8; XSRF-TOKEN=eyJpdiI6IkM4WHZ5bTQ3QnV5Y1V1NUZTMmRlZEE9PSIsInZhbHVlIjoiZDhRSmFldWMybWROMGZiVit2dGxYMFNPNkhvRHcvUENXUFpHZzl2VG15NDRNY1hiMTNmV3NJYzBib0VjT2VkY29Ld2dpVlFiRVN5L1dLRjd1aU41U251Y0xtbWFuVTlxVE9keThleEhKZ1ZPdFR0TmwyTDhwM2oxc1RkcG5tenMiLCJtYWMiOiI0YTliZmUyZWY1ZGEzNDIzNmViMDNiZDFjOWUyZTVhMGNiZDFhMWI4ZjA0YmJlNWNiY2Q4YjY2ODc5MDhlMjc0IiwidGFnIjoiIn0%3D; laravel_session=eyJpdiI6IlVicnRPR1pJb2ZTaGV0NjBnM09OdEE9PSIsInZhbHVlIjoid3dHQXEyaS9mbTJVdEhFWEdib29LcGx5ZVA4cW1aZm9PbW1qRlFpdlNCUURYTkViM2piUjVNdWVSYUQyMXVSYjhnQjI4UFhLQ1ZnbkVEemkrT3YzK3dmcFcxYkdMeUhRRDg0NUtCTXpZOE1ZQTFpS2NCSk1zUHA5SmxYdXlWbWMiLCJtYWMiOiI0MDA5Yzk0ZTMzZGMzZDVkZTQ5YTgzN2Q5ZGZhNTQ0YzJlYjg5MmNmMTZlMjIzMzczNjYxYzAwY2Y1NTA0ZDk2IiwidGFnIjoiIn0%3D
Connection: close

[johseg]

It existed at the time, but I don't have an environment to reproduce as I found this in a client environment

[streamtw]

@sw0rd1ight in v2.x of league/flysystem, exception PathTraversalDetected is thrown. As for versions before v2.x, I do not know for sure it there is any other mechanism that prevents directory traversal.

[sw0rd1ight]

According to the information I collected, this version seems to have vulnerabilities. Can you tell which version this is from the picture? @johseg @streamtw

[streamtw]

@sw0rd1ight The screenshot you provided looks like v1.x of this package.

[sw0rd1ight]

yes . I can reproduce this problem using unisharp/laravel-filemanager:1.8.0 laravel-filemanager: league/flysystem has not been introduced in 1.8.0.

<?php namespace Unisharp\Laravelfilemanager\controllers;

/**
 * Class DownloadController
 * @package Unisharp\Laravelfilemanager\controllers
 */
class DownloadController extends LfmController
{
    /**
     * Download a file
     *
     * @return mixed
     */
    public function getDownload()
    {
        return response()->download(parent::getCurrentPath(request('file')));
    }
}

Therefore, the scope of impact described by CVE-2022-40734 is not accurate.

[streamtw]

@sw0rd1ight Thanks for your effort in reproducing this issue.

In my opinion, users who visit CVE pages may want to know the exact version that will not encounter the security issues. In that case, I need to report to CVE that versions greater than or equal to v2.6.4 will do for sure (because v2.6.4 requires user to install v2.x of league/flysystem).

Although some of the previous versions may not encounter this issue, since user can choose to install v1.x or v2.x versions of league/flysystem, it is more easy to avoid this issue just by updating this package to v2.6.4 or newer versions. This is the reason why the current affected version range are marked in the CVE page.

Any suggestions?

[sw0rd1ight]

@streamtw ok.I have sent you a report to your email address streamtw.one@gmail.com