Skip to content

Commit

Permalink
Merge pull request from GHSA-qh4g-4m4w-jgv2
Browse files Browse the repository at this point in the history
Fix partial path traversal in zip file overwite protection
  • Loading branch information
nahsra authored Feb 1, 2024
2 parents 7c8e93e + 98bb779 commit b885b03
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 16 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,22 @@ In Maven:
<dependency>
<groupId>io.github.pixee</groupId>
<artifactId>java-security-toolkit</artifactId>
<version>1.1.1</version>
<version>1.1.2</version>
</dependency>
```
In Gradle:
```kotlin
implementation("io.github.pixee:java-security-toolkit:1.1.1")
implementation("io.github.pixee:java-security-toolkit:1.1.2")
```

## Contributing
We'd love to get contributions! See [CONTRIBUTING.md](CONTRIBUTING.md).

### Building
Building is meant for Java 11 and Maven 3:
Building is meant for Java 11:

```
mvn clean package
./gradlew check
```

## FAQ
Expand Down
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ tasks.named(java11SourceSet.jarTaskName) {
}

group = "io.github.pixee"
version = "1.1.1"
version = "1.1.2"
description = "java-security-toolkit"


Expand Down
17 changes: 10 additions & 7 deletions src/main/java/io/github/pixee/security/ZipSecurity.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;

Expand Down Expand Up @@ -67,9 +68,8 @@ public ZipEntry getNextEntry() throws IOException {

private boolean containsEscapesAndTargetsBelowRoot(final String name) {
if (name.contains("../") || name.contains("..\\")) {
final File fileWithEscapes = new File(name);
try {
if (isBelowCurrentDirectory(fileWithEscapes)) {
if (isBelowOrSisterToCurrentDirectory(name)) {
return true;
}
} catch (IOException e) {
Expand All @@ -79,11 +79,14 @@ private boolean containsEscapesAndTargetsBelowRoot(final String name) {
return false;
}

boolean isBelowCurrentDirectory(final File fileWithEscapes) throws IOException {
final File currentDirectory = new File("");
String canonicalizedTargetPath = fileWithEscapes.getCanonicalPath();
String canonicalizedCurrentPath = currentDirectory.getCanonicalPath();
return !canonicalizedTargetPath.startsWith(canonicalizedCurrentPath);
private boolean isBelowOrSisterToCurrentDirectory(final String untrustedFileWithEscapes) throws IOException {
// Get the absolute path of the current directory
final File currentDirectory = new File("").getCanonicalFile();
final Path currentPathRoot = currentDirectory.toPath();
// Get the absolute path of the untrusted file
final File untrustedFile = new File(currentDirectory, untrustedFileWithEscapes);
final Path pathWithEscapes = untrustedFile.getCanonicalFile().toPath();
return !pathWithEscapes.startsWith(currentPathRoot);
}

private boolean isRootFileEntry(final String name) {
Expand Down
18 changes: 14 additions & 4 deletions src/test/java/io/github/pixee/security/ZipSecurityTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
import static org.hamcrest.Matchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.*;
import java.nio.charset.StandardCharsets;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
Expand Down Expand Up @@ -50,6 +47,19 @@ void it_prevents_escapes(String path) throws IOException {
assertThrows(SecurityException.class, hardenedStream::getNextEntry);
}

/**
* This tests that there is no regression of CVE-2024-24569, which was a partial path traversal bypass that allowed access to the current working directory's sibling directories.
*/
@Test
void it_prevents_sister_directory_escape() throws IOException {
String currentDir = new File("").getCanonicalFile().getName();
ZipEntry entry = new ZipEntry("foo/../../" + currentDir + "-other-sister-dir");
InputStream is = createZipFrom(entry);

ZipInputStream hardenedStream = ZipSecurity.createHardenedInputStream(is);
assertThrows(SecurityException.class, hardenedStream::getNextEntry);
}

@Test
void it_prevents_absolute_paths_in_zip_entries() throws IOException {
ZipEntry entry = new ZipEntry("/foo.txt");
Expand Down

0 comments on commit b885b03

Please sign in to comment.