-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
Fix code scanning alerts #1061
Conversation
Cherry pick to beta failed, 1 conflicted file in commit 91e3b12
|
| 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
file system operation
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Unsanitized archive entry, which may contain '..', is used in a
file system operation
Show autofix suggestion
Hide autofix suggestion
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 eachSevenZArchiveEntry) anddecompressJar(for eachJarEntry), similar to how it is done (perhaps in an auxiliary method) for tar files.
In particular, the following regions require fixes:
- In
decompress7z, after creatingnewFile, validate it before any file system operation. - In
decompressJar, after creatingoutputFile, validate it before any file system operation.
Code imports required:
- Ensure that
java.nio.file.Pathis 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.
-
Copy modified lines R616-R620 -
Copy modified lines R779-R783
| @@ -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()) { |
codeql