Bug 535824 - Check for directory traversal entry pathsY20180712-0200I20180717-0320I20180716-2000I20180716-0715I20180715-2000I20180714-1500I20180713-2000I20180712-2000I20180711-2000I20180710-2000

Change-Id: I105a488ea8264c1a2420a819255e6a95219365ad
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

5 files changed, 63 insertions, 6 deletions

diff --git a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java
index 6f17bc0dd..8a5676632 100644
--- a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java
+++ b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java
@@ -25,7 +25,7 @@ public class FileUtils {
 			for (Enumeration<TarEntry> e = tarFile.entries(); e.hasMoreElements();) {
 				TarEntry entry = e.nextElement();
 				try (InputStream input = tarFile.getInputStream(entry)) {
-					File outFile = new File(outputDir, entry.getName());
+					File outFile = createSubPathFile(outputDir, entry.getName());
 					outFile = outFile.getCanonicalFile(); //bug 266844
 					untarredFiles.add(outFile);
 					if (entry.getFileType() == TarEntry.DIRECTORY) {
@@ -103,7 +103,7 @@ public class FileUtils {
 				throw new IOException(Messages.Util_Invalid_Zip_File_Format);
 			}
 			do {
-				File outFile = new File(outputDir, ze.getName());
+				File outFile = createSubPathFile(outputDir, ze.getName());
 				unzippedFiles.add(outFile);
 				if (ze.isDirectory()) {
 					outFile.mkdirs();
@@ -128,6 +128,16 @@ public class FileUtils {
 		return unzippedFiles.toArray(new File[unzippedFiles.size()]);
 	}
 
+	private static File createSubPathFile(File root, String subPath) throws IOException {
+		File result = new File(root, subPath);
+		String resultCanonical = result.getCanonicalPath();
+		String rootCanonical = root.getCanonicalPath();
+		if (!resultCanonical.startsWith(rootCanonical + File.separator) && !resultCanonical.equals(rootCanonical)) {
+			throw new IOException("Invalid path: " + subPath); //$NON-NLS-1$
+		}
+		return result;
+	}
+
 	// Delete empty directories under dir, including dir itself.
 	public static void deleteEmptyDirs(File dir) throws IOException {
 		File[] files = dir.listFiles();
diff --git a/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/equinox/internal/p2/jarprocessor/ZipProcessor.java b/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/equinox/internal/p2/jarprocessor/ZipProcessor.java
index 0b41efe81..92879e84b 100644
--- a/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/equinox/internal/p2/jarprocessor/ZipProcessor.java
+++ b/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/equinox/internal/p2/jarprocessor/ZipProcessor.java
@@ -83,7 +83,7 @@ public class ZipProcessor {
 						File extractedFile = null;
 
 						if (entry.getName().endsWith(extension) && (pack || sign || repack || options.unpack)) {
-							extractedFile = new File(tempDir, name);
+							extractedFile = createSubPathFile(tempDir, name);
 							parent = extractedFile.getParentFile();
 							if (!parent.exists())
 								parent.mkdirs();
@@ -192,6 +192,16 @@ public class ZipProcessor {
 
 	}
 
+	public static File createSubPathFile(File root, String subPath) throws IOException {
+		File result = new File(root, subPath);
+		String resultCanonical = result.getCanonicalPath();
+		String rootCanonical = root.getCanonicalPath();
+		if (!resultCanonical.startsWith(rootCanonical + File.separator) && !resultCanonical.equals(rootCanonical)) {
+			throw new IOException("Invalid path: " + subPath); //$NON-NLS-1$
+		}
+		return result;
+	}
+
 	private void initialize(ZipFile zip) {
 		ZipEntry entry = zip.getEntry("pack.properties"); //$NON-NLS-1$
 		properties = new Properties();
diff --git a/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/internal/provisional/equinox/p2/jarprocessor/JarProcessor.java b/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/internal/provisional/equinox/p2/jarprocessor/JarProcessor.java
index a73cf3b8c..fcc6a4d53 100644
--- a/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/internal/provisional/equinox/p2/jarprocessor/JarProcessor.java
+++ b/bundles/org.eclipse.equinox.p2.jarprocessor/src/org/eclipse/internal/provisional/equinox/p2/jarprocessor/JarProcessor.java
@@ -98,7 +98,7 @@ public class JarProcessor {
 				JarEntry newEntry = null;
 				if (replacements.containsKey(entry.getName())) {
 					String name = replacements.get(entry.getName());
-					replacement = new File(directory, name);
+					replacement = ZipProcessor.createSubPathFile(directory, name);
 					if (name != null) {
 						if (replacement.exists()) {
 							try {
@@ -196,7 +196,7 @@ public class JarProcessor {
 						System.out.println("Processing nested file: " + name); //$NON-NLS-1$
 					}
 					//extract entry to temp directory
-					File extracted = new File(tempDir, name);
+					File extracted = ZipProcessor.createSubPathFile(tempDir, name);
 					File parentDir = extracted.getParentFile();
 					if (!parentDir.exists())
 						parentDir.mkdirs();
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java
index 23ff71a75..10bcc7c41 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java
@@ -12,15 +12,19 @@
 package org.eclipse.equinox.p2.tests.core;
 
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.Enumeration;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Path;
 import org.eclipse.equinox.internal.p2.core.helpers.FileUtils;
 import org.eclipse.equinox.internal.p2.core.helpers.FileUtils.IPathComputer;
+import org.eclipse.equinox.internal.p2.touchpoint.natives.Util;
 import org.eclipse.equinox.p2.tests.AbstractProvisioningTest;
+import org.eclipse.equinox.p2.tests.TestActivator;
 
 /**
  * @since 3.5
@@ -83,6 +87,29 @@ public class FileUtilsTest extends AbstractProvisioningTest {
 
 	}
 
+	public void testUnzipEscapeZipRoot() throws IOException {
+		File badZip = TestActivator.getContext().getDataFile(getName() + ".zip");
+		try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(badZip))) {
+			zos.putNextEntry(new ZipEntry("../../escapeRoot.txt"));
+			zos.write("test data".getBytes());
+			zos.closeEntry();
+		}
+		File temp = getTempFolder();
+		try {
+			FileUtils.unzipFile(badZip, temp);
+		} catch (IOException e) {
+			// expected
+			assertTrue("Wrong message: " + e.getMessage(), e.getMessage().indexOf("Invalid path: ") >= 0);
+		}
+
+		try {
+			Util.unzipFile(badZip, temp, null, null, null);
+		} catch (IOException e) {
+			// expected
+			assertTrue("Wrong message: " + e.getMessage(), e.getMessage().indexOf("Invalid path: ") >= 0);
+		}
+	}
+
 	public void testZipRootPathComputer() {
 		File temp = getTempFolder();
 
diff --git a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Util.java b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Util.java
index 212c5571d..99a20f5aa 100644
--- a/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Util.java
+++ b/bundles/org.eclipse.equinox.p2.touchpoint.natives/src/org/eclipse/equinox/internal/p2/touchpoint/natives/Util.java
@@ -176,7 +176,7 @@ public class Util {
 									name = name.substring(1);
 							}
 						}
-						File outFile = new File(outputDir, name);
+						File outFile = createSubPathFile(outputDir, name);
 						unzippedFiles.add(outFile);
 						if (ze.isDirectory()) {
 							outFile.mkdirs();
@@ -206,6 +206,16 @@ public class Util {
 
 	}
 
+	private static File createSubPathFile(File root, String subPath) throws IOException {
+		File result = new File(root, subPath);
+		String resultCanonical = result.getCanonicalPath();
+		String rootCanonical = root.getCanonicalPath();
+		if (!resultCanonical.startsWith(rootCanonical + File.separator) && !resultCanonical.equals(rootCanonical)) {
+			throw new IOException("Invalid path: " + subPath); //$NON-NLS-1$
+		}
+		return result;
+	}
+
 	/**
 	 * Copy an input stream to an output stream.
 	 * Optionally close the streams when done.