This repository has been archived by the owner on Oct 23, 2024. It is now read-only.
[SECURITY] Fix Partial Path Traversal Vulnerability #3375
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Security Vulnerability Fix
This pull request fixes a partial-path traversal vulnerability due to an insufficient path traversal guard.
Even if you deem, as the maintainer of this project, this is not necessarily fixing a security vulnerability, it is still a valid security hardening.
Preamble
Impact
This issue allows a malicious actor to potentially break out of the expected directory. The impact is limited to sibling directories. For example,
userControlled.getCanonicalPath().startsWith("/usr/out")
will allow an attacker to access a directory with a name like/usr/outnot
.Why?
To demonstrate this vulnerability, consider
"/usr/outnot".startsWith("/usr/out")
.The check is bypassed although
/outnot
is not under the/out
directory.It's important to understand that the terminating slash may be removed when using various
String
representations of theFile
object.For example, on Linux,
println(new File("/var"))
will print/var
, butprintln(new File("/var", "/")
will print/var/
;however,
println(new File("/var", "/").getCanonicalPath())
will print/var
.The Fix
Comparing paths with the
java.nio.files.Path#startsWith
will adequately protect againts this vulnerability.For example:
file.getCanonicalFile().toPath().startsWith(BASE_DIRECTORY)
orfile.getCanonicalFile().toPath().startsWith(BASE_DIRECTORY_FILE.getCanonicalFile().toPath())
Other Examples
➡️ Vulnerability Disclosure ⬅️
👋 Vulnerability disclosure is a super important part of the vulnerability handling process and should not be skipped! This may be completely new to you, and that's okay, I'm here to assist!
First question, do we need to perform vulnerability disclosure? It depends!
For partial path traversal, consider if user-supplied input could ever flow to this logic. If user supplied input could reach this conditional, it's insufficient and, as such, most likely a vulnerability.
Vulnerability Disclosure How-To
You have a few options options to perform vulnerability disclosure. However, I'd like to suggest the following 2 options:
JLLeitschuh Disclosure
in the subject of your email so it is not missed.Detecting this and Future Vulnerabilities
You can automatically detect future vulnerabilities like this by enabling the free (for open-source) GitHub Action.
I'm not an employee of GitHub, I'm simply an open-source security researcher.
Source
This contribution was automatically generated with an OpenRewrite refactoring recipe, which was lovingly hand crafted to bring this security fix to your repository.
The source code that generated this PR can be found here:
PartialPathTraversalVulnerability
Why didn't you disclose privately (ie. coordinated disclosure)?
This PR was automatically generated, in-bulk, and sent to this project as well as many others, all at the same time.
This is technically what is called a "Full Disclosure" in vulnerability disclosure, and I agree it's less than ideal. If GitHub offered a way to create private pull requests to submit pull requests, I'd leverage it, but that infrastructure, sadly, doesn't exist yet.
The problem is that as an open source software security researcher, I (exactly like open source maintainers), I only have so much time in a day. I'm able to find vulnerabilities impacting hundreds, or sometimes thousands of open source projects with tools like GitHub Code Search and CodeQL. The problem is that my knowledge of vulnerabilities doesn't scale very well.
Individualized vulnerability disclosure takes time and care. It's a long and tedious process, and I have a significant amount of experience with it (I have over 50 CVEs to my name). Even tracking down the reporting channel (email, Jira, ect..) can take time and isn't automatable. Unfortunately, when facing prblems of this scale, individual reporting doesn't work well either.
Additionally, if I just spam out emails or issues, I'll just overwhelm already over taxed maintainers, I don't want to do this either.
By creating a pull request, I am aiming to provide maintainers something highly actionable to actually fix the identified vulnerability; a pull request.
There's a larger discussion on this topic that can be found here: JLLeitschuh/security-research#12
Opting-Out
If you'd like to opt-out of future automated security vulnerability fixes like this, please consider adding a file called
.github/GH-ROBOTS.txt
to your repository with the line:This bot will respect the ROBOTS.txt format for future contributions.
Alternatively, if this project is no longer actively maintained, consider archiving the repository.
CLA Requirements
This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.
It is unlikely that I'll be able to directly sign CLAs. However, all contributed commits are already automatically signed-off.
If signing your organization's CLA is a strict-requirement for merging this contribution, please feel free to close this PR.
Sponsorship & Support
This contribution is sponsored by HUMAN Security Inc. and the new Dan Kaminsky Fellowship, a fellowship created to celebrate Dan's memory and legacy by funding open-source work that makes the world a better (and more secure) place.
This PR was generated by Moderne, a free-for-open source SaaS offering that uses format-preserving AST transformations to fix bugs, standardize code style, apply best practices, migrate library versions, and fix common security vulnerabilities at scale.
Tracking
All PR's generated as part of this fix are tracked here: JLLeitschuh/security-research#13