Skip to content

Fix code scanning alerts #1061

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

Closed
sgrampone wants to merge 1 commit into from
Closed

Fix code scanning alerts #1061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions gxcompress/src/main/java/com/genexus/compression/CompressionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Enumeration;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
Expand Down Expand Up @@ -304,4 +306,13 @@ private static boolean isEntryPathSafe(File targetPath, String entryName) throws
File destinationFile = new File(targetPath, entryName).getCanonicalFile();
return destinationFile.getPath().startsWith(targetPath.getPath() + File.separator) || destinationFile.getPath().equals(targetPath.getPath());
}

public static boolean isPathTraversal(String dir, String fName) {
try {
Path path = Paths.get(dir).resolve(fName);
return !path.toAbsolutePath().equals(path.toRealPath());
}catch (Exception e){
return true;
}
}
}
33 changes: 19 additions & 14 deletions gxcompress/src/main/java/com/genexus/compression/GXCompressor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,20 +639,25 @@
try (TarArchiveInputStream tis = new TarArchiveInputStream(Files.newInputStream(archive.toPath()))) {
TarArchiveEntry entry;
while ((entry = tis.getNextEntry()) != null) {
File newFile = new File(directory, entry.getName());
if (entry.isDirectory()) {
if (!newFile.isDirectory() && !newFile.mkdirs()) {
throw new IOException("Failed to create directory " + newFile);
}
} else {
File parent = newFile.getParentFile();
if (!parent.isDirectory() && !parent.mkdirs()) {
throw new IOException("Failed to create directory " + parent);
}
try (OutputStream out = Files.newOutputStream(newFile.toPath())) {
int len;
while ((len = tis.read(buffer)) != -1) {
out.write(buffer, 0, len);
if(CompressionUtils.isPathTraversal(directory, entry.getName())){
log.error(DIRECTORY_ATTACK + "{}", entry.getName());
return;
}else {
File newFile = new File(directory, entry.getName());

Check failure

Code scanning / CodeQL

Arbitrary file access during archive extraction ("Zip Slip") High

Unsanitized archive entry, which may contain '..', is used in a
file system operation
Loading
.
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Loading
.
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Loading
.
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Loading
.
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Loading
.

Copilot Autofix

AI 14 days ago

To fix the Zip Slip vulnerability, we need to ensure that output paths created using an archive entry's name are securely validated before performing any file system operation. The best way to do this is:

  • For every extracted file (across tar, 7z, jar), construct the destination file path and normalize it.
  • Check whether the normalized path is under the intended extraction directory by comparing path prefixes (Path.startsWith() or equivalent using canonical path).
  • If the path does not start with the intended extraction directory, abort extraction of that entry and possibly log or throw an exception.
  • Implement this in both decompress7z (for each SevenZArchiveEntry) and decompressJar (for each JarEntry), similar to how it is done (perhaps in an auxiliary method) for tar files.

In particular, the following regions require fixes:

  • In decompress7z, after creating newFile, validate it before any file system operation.
  • In decompressJar, after creating outputFile, validate it before any file system operation.

Code imports required:

  • Ensure that java.nio.file.Path is imported (already is).
  • Use toPath().normalize() and compare with the extraction directory's canonical path.

If there's already a helper utility for path traversal check (CompressionUtils.isPathTraversal), use that for cross-format consistency. If not, implement a simple helper locally in this file.

Suggested changeset 1
gxcompress/src/main/java/com/genexus/compression/GXCompressor.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/gxcompress/src/main/java/com/genexus/compression/GXCompressor.java b/gxcompress/src/main/java/com/genexus/compression/GXCompressor.java
--- a/gxcompress/src/main/java/com/genexus/compression/GXCompressor.java
+++ b/gxcompress/src/main/java/com/genexus/compression/GXCompressor.java
@@ -613,6 +613,11 @@
 		try (SevenZFile sevenZFile = new SevenZFile(archive)) {
 			SevenZArchiveEntry entry;
 			while ((entry = sevenZFile.getNextEntry()) != null) {
+				// Added Zip Slip protection for 7z extraction
+				if (CompressionUtils.isPathTraversal(directory, entry.getName())) {
+					log.error(DIRECTORY_ATTACK + "{}", entry.getName());
+					continue;
+				}
 				File newFile = new File(directory, entry.getName());
 				if (entry.isDirectory()) {
 					if (!newFile.isDirectory() && !newFile.mkdirs()) {
@@ -771,6 +776,11 @@
 		try (JarInputStream jarInputStream = new JarInputStream(Files.newInputStream(archive.toPath()))) {
 			JarEntry entry;
 			while ((entry = jarInputStream.getNextJarEntry()) != null) {
+				// Added Zip Slip protection for jar extraction
+				if (CompressionUtils.isPathTraversal(targetDir.getAbsolutePath(), entry.getName())) {
+					log.error(DIRECTORY_ATTACK + "{}", entry.getName());
+					continue;
+				}
 				File outputFile = new File(targetDir, entry.getName());
 				if (entry.isDirectory()) {
 					if (!outputFile.exists() && !outputFile.mkdirs()) {
EOF
@@ -613,6 +613,11 @@
try (SevenZFile sevenZFile = new SevenZFile(archive)) {
SevenZArchiveEntry entry;
while ((entry = sevenZFile.getNextEntry()) != null) {
// Added Zip Slip protection for 7z extraction
if (CompressionUtils.isPathTraversal(directory, entry.getName())) {
log.error(DIRECTORY_ATTACK + "{}", entry.getName());
continue;
}
File newFile = new File(directory, entry.getName());
if (entry.isDirectory()) {
if (!newFile.isDirectory() && !newFile.mkdirs()) {
@@ -771,6 +776,11 @@
try (JarInputStream jarInputStream = new JarInputStream(Files.newInputStream(archive.toPath()))) {
JarEntry entry;
while ((entry = jarInputStream.getNextJarEntry()) != null) {
// Added Zip Slip protection for jar extraction
if (CompressionUtils.isPathTraversal(targetDir.getAbsolutePath(), entry.getName())) {
log.error(DIRECTORY_ATTACK + "{}", entry.getName());
continue;
}
File outputFile = new File(targetDir, entry.getName());
if (entry.isDirectory()) {
if (!outputFile.exists() && !outputFile.mkdirs()) {
Copilot is powered by AI and may make mistakes. Always verify output.
if (entry.isDirectory()) {
if (!newFile.isDirectory() && !newFile.mkdirs()) {
throw new IOException("Failed to create directory " + newFile);
}
} else {
File parent = newFile.getParentFile();
if (!parent.isDirectory() && !parent.mkdirs()) {
throw new IOException("Failed to create directory " + parent);
}
try (OutputStream out = Files.newOutputStream(newFile.toPath())) {
int len;
while ((len = tis.read(buffer)) != -1) {
out.write(buffer, 0, len);
}
}
}
}
Expand Down
Loading