This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release18.12 by this push:
new 01c0ff5 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.
01c0ff5 is described below
commit 01c0ff5469346fcce0c2d613026ca234c546f564
Author: Jacques Le Roux <[email protected]>
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 1ec7667..ec027c4 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
@@ -449,13 +449,33 @@ 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
@@ -464,15 +484,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();
@@ -487,6 +515,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))
}
