From f58e9a8c197ab342f6495e8becaf55876e479a01 Mon Sep 17 00:00:00 2001 From: Piotr Krysiak Date: Wed, 18 Jul 2018 14:35:48 +0200 Subject: Fix zip-slip in openecomp-be Issue-ID: SDC-1401 Change-Id: I92cf8184ab50cb1d3b1ba2f71eab8f5701e1ee57 Signed-off-by: Piotr Krysiak --- .../org/openecomp/sdc/common/utils/CommonUtil.java | 9 ++++- .../sdc/translator/utils/ResourceWalker.java | 2 +- .../org/openecomp/core/tools/util/ZipUtils.java | 47 +++++++++++++--------- 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java b/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java index 206eae3491..dfd6b8d250 100644 --- a/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java +++ b/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java @@ -47,6 +47,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.zip.ZipEntry; +import java.util.zip.ZipException; import java.util.zip.ZipInputStream; public class CommonUtil { @@ -95,8 +96,8 @@ public class CommonUtil { String currentEntryName; while ((zipEntry = inputZipStream.getNextEntry()) != null) { + assertEntryNotVulnerable(zipEntry); currentEntryName = zipEntry.getName(); - // else, get the file content (as byte array) and save it in a map. fileByteContent = FileUtils.toByteArray(inputZipStream); int index = lastIndexFileSeparatorIndex(currentEntryName); @@ -115,6 +116,12 @@ public class CommonUtil { return new ImmutablePair<>(mapFileContent, folderList); } + private static void assertEntryNotVulnerable(ZipEntry entry) throws ZipException { + if (entry.getName().contains("../")) { + throw new ZipException("Path traversal attempt discovered."); + } + } + private static boolean isFile(String currentEntryName) { return !(currentEntryName.endsWith("\\") || currentEntryName.endsWith("/")); } diff --git a/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java b/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java index e5993677cd..93a2290938 100644 --- a/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java +++ b/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java @@ -123,7 +123,7 @@ public class ResourceWalker { return zipEntry -> { String name = zipEntry.getName(); return (name.equals(resource) || name.startsWith(resource + "/")) - && !zipEntry.isDirectory(); + && !zipEntry.isDirectory() && !name.contains("../"); }; } } diff --git a/openecomp-be/tools/zusammen-tools/src/main/java/org/openecomp/core/tools/util/ZipUtils.java b/openecomp-be/tools/zusammen-tools/src/main/java/org/openecomp/core/tools/util/ZipUtils.java index 96c7f17084..a2ea76d308 100644 --- a/openecomp-be/tools/zusammen-tools/src/main/java/org/openecomp/core/tools/util/ZipUtils.java +++ b/openecomp-be/tools/zusammen-tools/src/main/java/org/openecomp/core/tools/util/ZipUtils.java @@ -13,6 +13,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.zip.ZipEntry; +import java.util.zip.ZipException; import java.util.zip.ZipInputStream; import java.util.zip.ZipOutputStream; @@ -45,33 +46,43 @@ public class ZipUtils { if (zipFile == null || outputFolder == null) { return; } - if (!outputFolder.toFile().exists()) { - Files.createDirectories(outputFolder); - } + createDirectoryIfNotExists(outputFolder); try (FileInputStream fileInputStream = new FileInputStream(zipFile.toFile()); - ZipInputStream zis = new ZipInputStream(fileInputStream)) { - ZipEntry ze = zis.getNextEntry(); - while (ze != null) { - String fileName = ze.getName(); + ZipInputStream stream = new ZipInputStream(fileInputStream)) { + + ZipEntry entry; + while ((entry = stream.getNextEntry()) != null) { + assertEntryNotVulnerable(entry); + String fileName = entry.getName(); File newFile = new File(outputFolder.toString() + File.separator + fileName); - if (ze.isDirectory()) { - Path path = newFile.toPath(); - if (!path.toFile().exists()) { - Files.createDirectories(path); - } + if (entry.isDirectory()) { + createDirectoryIfNotExists(newFile.toPath()); } else { - new File(newFile.getParent()).mkdirs(); - try (FileOutputStream fos = new FileOutputStream(newFile)) { - ByteStreams.copy(zis, fos); - } + persistFile(stream, newFile); } - ze = zis.getNextEntry(); } + } + + } - zis.closeEntry(); + private static void persistFile(ZipInputStream stream, File newFile) throws IOException { + new File(newFile.getParent()).mkdirs(); + try (FileOutputStream outputStream = new FileOutputStream(newFile)) { + ByteStreams.copy(stream, outputStream); } + } + private static void createDirectoryIfNotExists(Path path) throws IOException { + if (!path.toFile().exists()) { + Files.createDirectories(path); + } + } + + private static void assertEntryNotVulnerable(ZipEntry entry) throws ZipException { + if (entry.getName().contains("../")) { + throw new ZipException("Path traversal attempt discovered."); + } } } -- cgit 1.2.3-korg