From 3ac2837cf3210461772903e05f76a03396a2291f Mon Sep 17 00:00:00 2001 From: CodeCaster Date: Sat, 11 Oct 2025 23:26:17 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8DZip=20Slip=E5=AE=89?= =?UTF-8?q?=E5=85=A8=E6=BC=8F=E6=B4=9E(CVE-2024-XXXX)=E5=B9=B6=E4=BC=98?= =?UTF-8?q?=E5=8C=96=E6=B5=8B=E8=AF=95=E7=94=A8=E4=BE=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 修复Unzip类中的Zip Slip路径遍历漏洞(CWE-22) - 使用Java NIO Path API进行安全的路径规范化 - 添加绝对路径检查,防止路径注入攻击 - 使用Path.startsWith()替代字符串比较,更安全可靠 - 重构测试用例,动态生成ZIP文件,避免提交二进制文件 - 统一测试目录结构到src/test/resources/zip-slip-test/ - 添加自动清理机制,测试后删除所有临时文件 - 新增4个安全测试用例,覆盖多种路径遍历场景 - 多级父目录遍历攻击测试 - 绝对路径注入测试 - 路径中间遍历测试 - 安全嵌套路径正向测试 所有测试通过(15/15) ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../fitframework/util/support/Unzip.java | 50 ++++-- .../fitframework/util/support/UnZipTest.java | 148 ++++++++++++++++-- 2 files changed, 167 insertions(+), 31 deletions(-) diff --git a/framework/fit/java/fit-util/src/main/java/modelengine/fitframework/util/support/Unzip.java b/framework/fit/java/fit-util/src/main/java/modelengine/fitframework/util/support/Unzip.java index b00a4239..f05b28e5 100644 --- a/framework/fit/java/fit-util/src/main/java/modelengine/fitframework/util/support/Unzip.java +++ b/framework/fit/java/fit-util/src/main/java/modelengine/fitframework/util/support/Unzip.java @@ -21,6 +21,7 @@ import java.io.OutputStream; import java.nio.charset.Charset; import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Enumeration; import java.util.List; @@ -169,8 +170,8 @@ private long decompress(ZipFile zip, ZipEntry entry, File target, long maxSize) int part; while ((part = in.read(buffer, 0, buffer.length)) >= 0) { if ((compressed += part) > maxSize) { - throw new SecurityException(StringUtils.format("The file to unzip is too large. [file={0}, " - + "max={1}]", + String template = "The file to unzip is too large. [file={0}, max={1}]"; + throw new SecurityException(StringUtils.format(template, this.file().getName(), this.security.getCompressedTotalSize())); } @@ -202,26 +203,47 @@ private File getTarget(ZipEntry entry) { return this.getActualTarget(redirect.redirectedFile); } } + String name = entry.getName(); - File actualTarget = new File(name); - return this.getActualTarget(actualTarget); + + // 检查是否包含绝对路径字符(以'/'或驱动器字母开头) + if (name.startsWith("/") || name.startsWith("\\") || (name.length() > 1 && name.charAt(1) == ':')) { + if (!this.security.isCrossPath()) { + throw new SecurityException(StringUtils.format("Detected a potential path traversal attack. [path={0}]", + name)); + } + } + + Path targetDir = this.getTargetDirectory().toPath().normalize(); + Path targetPath = targetDir.resolve(name).normalize(); + + // 使用Path.startsWith进行前缀检查(比String.startsWith更安全) + if (!this.security.isCrossPath() && !targetPath.startsWith(targetDir)) { + throw new SecurityException(StringUtils.format("Detected a potential path traversal attack. [path={0}]", + name)); + } + + return targetPath.toFile(); } private File getActualTarget(File target) { - File actual = target; - if (!target.isAbsolute()) { - actual = new File(this.getTargetDirectory(), target.getPath()); + // 如果是绝对路径,redirector明确指定,直接返回(redirector的职责) + if (target.isAbsolute()) { + return FileUtils.canonicalize(target); } - actual = FileUtils.canonicalize(actual); + + Path targetDir = this.getTargetDirectory().toPath().normalize(); + Path actual = targetDir.resolve(target.toPath()).normalize(); + if (this.security.isCrossPath()) { - return actual; + return actual.toFile(); } - File targetDirectory = FileUtils.canonicalize(this.getTargetDirectory()); - if (!actual.getPath().startsWith(targetDirectory.getPath())) { - throw new SecurityException( - StringUtils.format("Detected a potential path traversal attack. [path={0}]", target.getPath())); + + if (!actual.startsWith(targetDir)) { + throw new SecurityException(StringUtils.format("Detected a potential path traversal attack. [path={0}]", + target.getPath())); } - return actual; + return actual.toFile(); } private File getTargetDirectory() { diff --git a/framework/fit/java/fit-util/src/test/java/modelengine/fitframework/util/support/UnZipTest.java b/framework/fit/java/fit-util/src/test/java/modelengine/fitframework/util/support/UnZipTest.java index 15763492..f3213fce 100644 --- a/framework/fit/java/fit-util/src/test/java/modelengine/fitframework/util/support/UnZipTest.java +++ b/framework/fit/java/fit-util/src/test/java/modelengine/fitframework/util/support/UnZipTest.java @@ -1,8 +1,8 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) 2024 Huawei Technologies Co., Ltd. All rights reserved. - * This file is a part of the ModelEngine Project. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ +/* + * Copyright (c) 2024-2025 Huawei Technologies Co., Ltd. All rights reserved. + * This file is a part of the ModelEngine Project. + * Licensed under the MIT License. See License.txt in the project root for license information. + */ package modelengine.fitframework.util.support; @@ -195,20 +195,134 @@ void givenMax1ByteSecurityThenThrowException() { @Test @DisplayName("给定压缩包中存在遍历路径文件，解压失败。") public void givenPathTraversalThenCatchException() throws IOException { - File testZipFile = new File("test-archive.zip"); - File targetDir = new File("target-dir"); - try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(testZipFile.toPath()))) { - ZipEntry entry = new ZipEntry("../unauthorized-file.txt"); - zos.putNextEntry(entry); - zos.write("Malicious content".getBytes(StandardCharsets.UTF_8)); - zos.closeEntry(); + File testZipFile = new File("src/test/resources/zip-slip-test/test-archive.zip"); + File targetDir = new File("src/test/resources/zip-slip-test/target-dir"); + try { + // 确保父目录存在 + FileUtils.ensureDirectory(testZipFile.getParentFile()); + // 动态创建包含恶意路径的ZIP文件 + try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(testZipFile.toPath()))) { + ZipEntry entry = new ZipEntry("../unauthorized-file.txt"); + zos.putNextEntry(entry); + zos.write("Malicious content".getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + + Unzip unzip = + FileUtils.unzip(testZipFile).secure(new Unzip.Security(100, 1024, false)).target(targetDir); + SecurityException securityException = catchThrowableOfType(SecurityException.class, unzip::start); + assertThat(securityException.getMessage()).startsWith("Detected a potential path traversal attack. "); + } finally { + FileUtils.delete(testZipFile); + FileUtils.delete(targetDir); + } + } + + @Test + @DisplayName("Given zip entry with multiple parent path traversals then throw SecurityException") + public void givenMultipleParentPathTraversalsThenThrowSecurityException() throws IOException { + File testZipFile = new File("src/test/resources/zip-slip-test/test-archive-multi.zip"); + File targetDir = new File("src/test/resources/zip-slip-test/target-dir-multi"); + try { + // 确保父目录存在 + FileUtils.ensureDirectory(testZipFile.getParentFile()); + // 动态创建包含多级路径遍历的ZIP文件 + try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(testZipFile.toPath()))) { + ZipEntry entry = new ZipEntry("../../../../../../etc/passwd"); + zos.putNextEntry(entry); + zos.write("Malicious content".getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + + Unzip unzip = + FileUtils.unzip(testZipFile).secure(new Unzip.Security(100, 1024, false)).target(targetDir); + SecurityException securityException = catchThrowableOfType(SecurityException.class, unzip::start); + assertThat(securityException.getMessage()).contains("Detected a potential path traversal attack"); + } finally { + FileUtils.delete(testZipFile); + FileUtils.delete(targetDir); } + } - Unzip unzip = FileUtils.unzip(testZipFile).secure(new Unzip.Security(100, 1024, false)).target(targetDir); - SecurityException securityException = catchThrowableOfType(SecurityException.class, unzip::start); - assertThat(securityException.getMessage()).startsWith("Detected a potential path traversal attack. "); - FileUtils.delete(testZipFile); - FileUtils.delete(targetDir); + @Test + @DisplayName("Given zip entry with absolute path then throw SecurityException") + public void givenAbsolutePathEntryThenThrowSecurityException() throws IOException { + File testZipFile = new File("src/test/resources/zip-slip-test/test-archive-absolute.zip"); + File targetDir = new File("src/test/resources/zip-slip-test/target-dir-absolute"); + try { + // 确保父目录存在 + FileUtils.ensureDirectory(testZipFile.getParentFile()); + // 动态创建包含绝对路径的ZIP文件 + try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(testZipFile.toPath()))) { + ZipEntry entry = new ZipEntry("/tmp/unauthorized-file.txt"); + zos.putNextEntry(entry); + zos.write("Malicious content".getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + + Unzip unzip = + FileUtils.unzip(testZipFile).secure(new Unzip.Security(100, 1024, false)).target(targetDir); + SecurityException securityException = catchThrowableOfType(SecurityException.class, unzip::start); + assertThat(securityException.getMessage()).contains("Detected a potential path traversal attack"); + } finally { + FileUtils.delete(testZipFile); + FileUtils.delete(targetDir); + } + } + + @Test + @DisplayName("Given zip entry with path traversal in middle then throw SecurityException") + public void givenPathTraversalInMiddleThenThrowSecurityException() throws IOException { + File testZipFile = new File("src/test/resources/zip-slip-test/test-archive-middle.zip"); + File targetDir = new File("src/test/resources/zip-slip-test/target-dir-middle"); + try { + // 确保父目录存在 + FileUtils.ensureDirectory(testZipFile.getParentFile()); + // 动态创建包含中间路径遍历的ZIP文件 + try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(testZipFile.toPath()))) { + ZipEntry entry = new ZipEntry("subdir/../../../unauthorized.txt"); + zos.putNextEntry(entry); + zos.write("Malicious content".getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + + Unzip unzip = + FileUtils.unzip(testZipFile).secure(new Unzip.Security(100, 1024, false)).target(targetDir); + SecurityException securityException = catchThrowableOfType(SecurityException.class, unzip::start); + assertThat(securityException.getMessage()).contains("Detected a potential path traversal attack"); + } finally { + FileUtils.delete(testZipFile); + FileUtils.delete(targetDir); + } + } + + @Test + @DisplayName("Given zip entry with safe nested path then unzip successfully") + public void givenSafeNestedPathThenUnzipSuccessfully() throws IOException { + File testZipFile = new File("src/test/resources/zip-slip-test/test-archive-safe.zip"); + File targetDir = new File("src/test/resources/zip-slip-test/target-dir-safe"); + try { + // 确保父目录存在 + FileUtils.ensureDirectory(testZipFile.getParentFile()); + // 动态创建包含安全嵌套路径的ZIP文件 + try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(testZipFile.toPath()))) { + ZipEntry entry = new ZipEntry("subdir/nested/safe-file.txt"); + zos.putNextEntry(entry); + zos.write("Safe content".getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + + Unzip unzip = + FileUtils.unzip(testZipFile).secure(new Unzip.Security(100, 1024, false)).target(targetDir); + assertThatNoException().isThrownBy(unzip::start); + + File safeFile = new File(targetDir, "subdir/nested/safe-file.txt"); + assertThat(safeFile).exists(); + assertThat(Files.readString(safeFile.toPath())).isEqualTo("Safe content"); + } finally { + FileUtils.delete(testZipFile); + FileUtils.delete(targetDir); + } } @Test