-
Notifications
Y 8000 ou must be signed in to change notification settings - Fork 731
Directory traversal vulnerability (CVE-2022-40734) #1150
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
Comments
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: 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. |
Just protected lfm routes with middleware [auth] until this issue fixed. |
This was assigned CVE-2022-40734. @silicahd: Can you please change the title to something like "Directory traversal vulnerability (CVE-2022-40734)"? |
…+00:00" Original commit: "FriendsOfPHP/security-advisories@8b09a24"
@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. |
…-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
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
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 |
So, should we consider this issue resolved and tag a new release ? |
Is this issue solved? |
bump |
Thank you @Jacotheron for your detailed elaboration! Looks like the issue will not exist if the version of I am finding a way to resolve the CVE. |
I have added dependency for |
I have addressed the patched version I will still try to update information on other CVE websites. But I think it is fine to mark this issue as solved. |
Does this vulnerability really exist? I can’t reproduce @johseg @streamtw
|
It existed at the time, but I don't have an environment to reproduce as I found this in a client environment |
@sw0rd1ight in |
@sw0rd1ight The screenshot you provided looks like |
yes .
Therefore, the scope of impact described by CVE-2022-40734 is not accurate. |
@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 Although some of the previous versions may not encounter this issue, since user can choose to install Any suggestions? |
@streamtw ok.I have sent you a report to your email address streamtw.one@gmail.com |
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.
The text was updated successfully, but these errors were encountered: