From 4ab9cbeee60820367f035e39e3d509c278d46632 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Tue, 10 Jul 2018 11:08:49 -0500 Subject: Bug 535824 - Check for directory traversal entry paths Change-Id: I105a488ea8264c1a2420a819255e6a95219365ad Signed-off-by: Thomas Watson --- .../internal/p2/core/helpers/FileUtils.java | 14 +++++++++-- .../internal/p2/jarprocessor/ZipProcessor.java | 12 +++++++++- .../equinox/p2/jarprocessor/JarProcessor.java | 4 ++-- .../equinox/p2/tests/core/FileUtilsTest.java | 27 ++++++++++++++++++++++ .../internal/p2/touchpoint/natives/Util.java | 12 +++++++++- 5 files changed, 63 insertions(+), 6 deletions(-) (limited to 'bundles') 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 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. -- cgit v1.2.3