You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/09/09 03:01:53 UTC

[GitHub] [maven-scm] JLLeitschuh opened a new pull request, #162: [SECURITY] Fix Partial Path Traversal Vulnerability

JLLeitschuh opened a new pull request, #162:
URL: https://github.com/apache/maven-scm/pull/162

   # 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 the `File` object.
   For example, on Linux, `println(new File("/var"))` will print `/var`, but `println(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)` or `file.getCanonicalFile().toPath().startsWith(BASE_DIRECTORY_FILE.getCanonicalFile().toPath())`
   
   ### Other Examples
   
    - [CVE-2022-31159](https://github.com/aws/aws-sdk-java/security/advisories/GHSA-c28r-hw5m-5gv3) - aws/aws-sdk-java
    - [CVE-2022-23457](https://securitylab.github.com/advisories/GHSL-2022-008_The_OWASP_Enterprise_Security_API/) - ESAPI/esapi-java-legacy
   
   # :arrow_right: Vulnerability Disclosure :arrow_left:
   
   :wave: 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!
   
    1. Is the vulnerable code only in tests or example code? No disclosure required!
    2. Is the vulnerable code in code shipped to your end users? Vulnerability disclosure is probably required!
   
   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:
   
    1. Request a CVE number from GitHub by creating a repository-level [GitHub Security Advisory](https://docs.github.com/en/code-security/repository-security-advisories/creating-a-repository-security-advisory). This has the advantage that, if you provide sufficient information, GitHub will automatically generate Dependabot alerts for your downstream consumers, resolving this vulnerability more quickly.
    2. Reach out to the team at Snyk to assist with CVE issuance. They can be reached at the [Snyk's Disclosure Email](mailto:report@snyk.io). Note: Please include `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](https://github.com/github/codeql-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](https://github.com/openrewrite/rewrite) [refactoring recipe](https://docs.openrewrite.org/), 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](https://github.com/openrewrite/rewrite-java-security/blob/main/src/main/java/org/openrewrite/java/security/PartialPathTraversalVulnerability.java)
   
   ## 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:  https://github.com/JLLeitschuh/security-research/discussions/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:
   
   ```
   User-agent: JLLeitschuh/security-research
   Disallow: *
   ```
   
   This bot will respect the [ROBOTS.txt](https://moz.com/learn/seo/robotstxt) format for future contributions.
   
   Alternatively, if this project is no longer actively maintained, consider [archiving](https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-archiving-repositories) 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.
   
   > The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin 
   > (see [https://developercertificate.org/](https://developercertificate.org/) for more information).
   >
   > \- [Git Commit Signoff documentation](https://developercertificate.org/)
   
   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](https://www.moderne.io/), 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: https://github.com/JLLeitschuh/security-research/issues/13


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] michael-o commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1242216132

   > By closing this PR are you communicating that do the maintainers not consider this a vulnerability for this project? I respect if that's your decision, but I'd like to know your rationale if you're willing to provide it.
   
   No, we expect report through official ASF channel.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] JLLeitschuh commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
JLLeitschuh commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1242181481

   By closing this PR are you communicating that do the maintainers not consider this a vulnerability for this project? I respect if that's your decision, but I'd like to know your rationale if you're willing to provide it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] olamy closed pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
olamy closed pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability
URL: https://github.com/apache/maven-scm/pull/162


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] olamy commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
olamy commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1241490271

   Please follow the Apache process regarding reporting possible security issue [here](https://www.apache.org/security/#:~:text=Send%20reports%20of%20vulnerabilities%20in,each%20vulnerability%20you%20are%20reporting.)
   
   It's not a good idea and definitely a very bad practice to report this in a public repository!!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] raboof commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
raboof commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1246413289

   Hi @JLLeitschuh,
   
   First of all, thanks for providing this PR and pointing out this potential issue.
   
   While as you know we generally prefer security issues to be reported privately to security@apache.org (or a project-specific security list), I can totally see how this might not be practical when mass-submitting issues found though static source code analysis. I respect that you only have so much time in a day, and appreciate you don't intend to overwhelm already over-taxed maintainers.
   
   Nonetheless, mass-creating issues is still putting the burden of doing the work needed
   to determine whether this is a legitimate security issue or a false positive on the maintainers. For reports on outdated dependencies with open CVE's, we don't accept such reports as security reports anymore, and ask reporters to either (a) do additional analysis, or (b) provide a public (non-security) issue/PR to update the version. We might apply the same logic to static code analysis tool output (though those can of course be more sophisticated) - so I'm happy to see you indeed provided this report in the form of a PR.
   
   I couldn't resist having a bit of a look, and at first glance it looks like a false positive from a security perspective: it's not obvious how `files` would contain any files in 'sibling' directories, and even if it did, it's unclear how that would lead to a problem.
   
   Still, possibly using `Path.startsWith` would be nice in general, and be a form of 'hardening' and avoiding unintended behavior that might be worth it regardless of security impact. In this
   particular case, however, it would likely require additional changes to the code to make sense, so I think rejecting the PR was a reasonable decision.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] JLLeitschuh commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
JLLeitschuh commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1241877688

   Hi @olamy,
   
   Normally I would, but in this case, I'm fixing vulnerabilities at scale and this is impractical.
   
   Please see the section above titled "Why didn't you disclose privately (ie. coordinated disclosure)?"


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] olamy commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
olamy commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1241880941

   please follow the defined Apache process and please avoid vulnerability disclosure....


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] JLLeitschuh commented on pull request #162: [SECURITY] Fix Partial Path Traversal Vulnerability

Posted by GitBox <gi...@apache.org>.
JLLeitschuh commented on PR #162:
URL: https://github.com/apache/maven-scm/pull/162#issuecomment-1242417432

   > No, we expect report through official ASF channel.
   
   Unfortunately, due to the amount of time this takes, this is not possible for me to do across every repository. I created 47 OSS pull requests to fix this vulnerability across the java ecosystem. Similarly, I opened 115 pull requests to fix Zip Slip across the java ecosystem. It's impractical for me to manually report this vulnerability to every single project that is impacted. I have a long history of disclosing to the ASF, it is a process that I'm familiar with. However when attempting to fix vulnerabilities at-scale what you are requesting is not possible.
   
   Has an internal ticket been created or has this issue been just closed until someone follows the "proper process"? 
   
   CC: @asfsecurity


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org