Chen keinan feat new cli support#182
Conversation
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
Signed-off-by: Chen Keinan <hen.keinan@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| byte[] buffer = new byte[4096]; | ||
|
|
||
| while ((entry = tarIn.getNextTarEntry()) != null) { | ||
| Path entryPath = destDir.resolve(entry.getName()); |
Check failure
Code scanning / CodeQL
Arbitrary file access during archive extraction ("Zip Slip") High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
In general, to fix Zip Slip / tar-slip vulnerabilities, you must validate that any path created from an archive entry name stays within the intended destination directory. This is typically done by resolving the entry path against the destination directory, normalizing it (or using canonical paths), and then checking that the resulting path starts with the destination directory path. If it does not, you should reject the entry (e.g., throw an exception or skip it).
For this specific code, the best low-impact fix is:
- After computing
entryPath = destDir.resolve(entry.getName());, normalize the path (e.g.,entryPath.normalize()). - Check that the normalized path starts with
destDir.normalize()usingPath.startsWith(...). UsingPathrather than string operations ensures segment-wise matching. - If the check fails, throw an
IOExceptionindicating a bad entry, and do not create directories or files for that entry. - Use the validated, normalized
entryPathfor subsequent filesystem operations (Files.createDirectories(...),Files.newOutputStream(...)).
This change should be implemented within extractTarGzResource in octopus-agent/src/main/java/octopus/teamcity/agent/EmbeddedResourceExtractor.java, right after line 154 where entryPath is currently defined. No new external dependencies are required; everything can be done with java.nio.file.Path, which is already imported. The functional behavior for valid archives remains the same, while malicious or malformed entries are now rejected.
| @@ -151,8 +151,13 @@ | ||
| byte[] buffer = new byte[4096]; | ||
|
|
||
| while ((entry = tarIn.getNextTarEntry()) != null) { | ||
| Path entryPath = destDir.resolve(entry.getName()); | ||
| Path entryPath = destDir.resolve(entry.getName()).normalize(); | ||
| Path normalizedDestDir = destDir.toAbsolutePath().normalize(); | ||
|
|
||
| if (!entryPath.toAbsolutePath().normalize().startsWith(normalizedDestDir)) { | ||
| throw new IOException("Entry is outside of the target dir: " + entry.getName()); | ||
| } | ||
|
|
||
| if (entry.isDirectory()) { | ||
| Files.createDirectories(entryPath); | ||
| } else { |
| String folderPrefix = resourceFolder.endsWith("/") ? resourceFolder : resourceFolder + "/"; | ||
|
|
||
| return jarFile.stream() | ||
| .map(JarEntry::getName) |
Check failure
Code scanning / CodeQL
Arbitrary file access during archive extraction ("Zip Slip") High
Copilot Autofix
AI 5 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
|
Duplicate to #183 |
3 participants
in repo copy of Chen's PR