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.

Sign up for GitHub

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

Jump to bottom

Fix Zip Slip Vulnerability in test code. #18

Closed
reckart opened this issue Feb 26, 2023 · 0 comments
Closed

Fix Zip Slip Vulnerability in test code. #18

reckart opened this issue Feb 26, 2023 · 0 comments
Assignees
@reckart
Labels
refactoring
Milestone
0.5.0

Comments

@reckart
Copy link
Member

reckart commented Feb 26, 2023

Describe the refactoring action
Fix the "Fix Zip Slip Vulnerability" in test code.

Expected benefit
No longer show up on security scans. Since this potential security issue only appears in the unit test code, it does not really affect any users. Cleaning this up is mostly cosmetic as it seems quite unlikely that somebody would exploit it to attack a developer's machine.

See: #10
@reckart reckart added this to the 0.5.0 milestone Feb 26, 2023
@reckart reckart self-assigned this Feb 26, 2023
reckart added a commit that referenced this issue Feb 26, 2023
@JLLeitschuh @TeamModerne @reckart
vuln-fix: Zip Slip Vulnerability (#10)
f7352f5 
Issue #18: Fix Zip Slip Vulnerability in test code

This change does one of two things. This change either

1. Inserts a guard to protect against Zip Slip.
OR
2. Replaces `dir.getCanonicalPath().startsWith(parent.getCanonicalPath())`, which is vulnerable to partial path traversal attacks, with the more secure `dir.getCanonicalFile().toPath().startsWith(parent.getCanonicalFile().toPath())`.

For number 2, 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`.

Weakness: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
Severity: High
CVSSS: 7.4
Detection: CodeQL (https://codeql.github.com/codeql-query-help/java/java-zipslip/) & OpenRewrite (https://public.moderne.io/recipes/org.openrewrite.java.security.ZipSlip)

Reported-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>


Bug-tracker: JLLeitschuh/security-research#16

Co-authored-by: Moderne <team@moderne.io>
Co-authored-by: Richard Eckart de Castilho <rec@apache.org>
@reckart reckart closed this as completed Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@reckart reckart

Labels
refactoring
Projects
None yet
Milestone
0.5.0
Development

No branches or pull requests
1 participant
@reckart