This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch trunk
in repository
https://gitbox.apache.org/repos/asf/ofbiz-framework.gitThe following commit(s) were added to refs/heads/trunk by this push:
new e136cb1 Fixed: Prevent Zip Slip vulnerability (OFBIZ-12056) While working with FileUtil::unzipFileToFolder I noticed that it's vulnerable to Zip slip vulnerability:
https://snyk.io/research/zip-slip-vulnerability.
e136cb1 is described below
commit e136cb1d9885fc6e0910637542308a9b7c10eb9f
Author: Jacques Le Roux <
[hidden email]>
AuthorDate: Sat Nov 14 11:39:10 2020 +0100
Fixed: Prevent Zip Slip vulnerability (OFBIZ-12056)
While working with FileUtil::unzipFileToFolder I noticed that it's vulnerable to
Zip slip vulnerability:
https://snyk.io/research/zip-slip-vulnerability.
Fortunately OOTB code does not use FileUtil::unzipFileToFolder so I did not
create a CVE, nor reported to
https://github.com/snyk/zip-slip-vulnerability#user-content-projects-affected-and-fixed.
If you think we should please shime in...
---
.../java/org/apache/ofbiz/base/util/FileUtil.java | 41 ++++++++++++++++++----
.../apache/ofbiz/base/util/FileUtilTests.groovy | 4 +--
2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
index 5c06cdd..7d08226 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
@@ -370,12 +370,32 @@ public final class FileUtil {
}
/**
+ * This useful to prevents Zip slip vulnerability cf.
https://snyk.io/research/zip-slip-vulnerability+ * @param destinationDirName The file path name to normalize
+ * @param zipEntry Zip entry to check before creating
+ * @return A file in destinationDir
+ */
+ public static File newFile(String destinationDirName, ZipEntry zipEntry) throws IOException {
+ File destinationDir = new File(destinationDirName);
+
+ File destFile = new File(destinationDir, zipEntry.getName());
+ String destDirPath = destinationDir.getCanonicalPath();
+ String destFilePath = destFile.getCanonicalPath();
+
+ if (!destFilePath.startsWith(destDirPath + File.separator)) {
+ throw new IOException("Entry is outside of the target dir: " + zipEntry.getName());
+ }
+ return destFile;
+ }
+
+ /**
* Unzip file structure of the given zipFile to specified outputFolder
* @param zipFile
* @param outputFolder
+ * @param handleZipSlip if true FileUtil::newFile is used
* @throws IOException
*/
- public static void unzipFileToFolder(File zipFile, String outputFolder) throws IOException {
+ public static boolean unzipFileToFolder(File zipFile, String outputFolder, boolean handleZipSlip) throws IOException {
byte[] buffer = new byte[8192];
//create output directory if not exists
@@ -384,15 +404,23 @@ public final class FileUtil {
folder.mkdir();
}
- //get the zip file content
+ // get the Zip file content
ZipInputStream zis = new ZipInputStream(new FileInputStream(zipFile));
- //get the zipped file list entry
+ // get the zipped file list entry
ZipEntry ze = zis.getNextEntry();
while (ze != null) {
- String fileName = ze.getName();
- File newFile = new File(outputFolder, fileName);
-
+ File newFile = null;
+ if (handleZipSlip) {
+ newFile = newFile(outputFolder, ze); // Prevents Zip slip vulnerability
+ if (null == newFile) {
+ zis.closeEntry();
+ zis.close();
+ return false;
+ }
+ } else {
+ newFile = new File(outputFolder, ze.getName());
+ }
//create all non existing folders
//else you will hit FileNotFoundException for compressed folder
new File(newFile.getParent()).mkdirs();
@@ -407,6 +435,7 @@ public final class FileUtil {
}
zis.closeEntry();
zis.close();
+ return true;
}
/**
diff --git a/framework/base/src/test/groovy/org/apache/ofbiz/base/util/FileUtilTests.groovy b/framework/base/src/test/groovy/org/apache/ofbiz/base/util/FileUtilTests.groovy
index ff39594..d9a2abd 100644
--- a/framework/base/src/test/groovy/org/apache/ofbiz/base/util/FileUtilTests.groovy
+++ b/framework/base/src/test/groovy/org/apache/ofbiz/base/util/FileUtilTests.groovy
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.ofbiz.base.util;
+package org.apache.ofbiz.base.util
import org.apache.commons.io.FileUtils
import org.junit.Test
@@ -55,7 +55,7 @@ public class FileUtilTests {
if (readme.exists()) readme.delete()
//validate unzip and compare the two files
- FileUtil.unzipFileToFolder(readmeZipped, zipFilePath)
+ FileUtil.unzipFileToFolder(readmeZipped, zipFilePath, false)
assert FileUtils.contentEquals(originalReadme, new File(zipFilePath, fileName))
}
Locked
