From 2c660df89f34884d8fb27eebbaa12df68cc3f2e8 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Wed, 13 Jun 2018 09:02:54 -0500 Subject: Bug 535822 - Check for directory traversal entry paths Change-Id: Ic989745f1115d3c23866957519f1d9fc8f315640 Signed-off-by: Thomas Watson --- .../tests/bundles/BundleInstallUpdateTests.java | 70 ++++++++++++++++++++-- .../src/org/eclipse/osgi/storage/BundleInfo.java | 31 +++++++--- .../src/org/eclipse/osgi/storage/Storage.java | 50 +++++++++++++++- .../osgi/storage/bundlefile/ZipBundleFile.java | 13 ++-- 4 files changed, 142 insertions(+), 22 deletions(-) (limited to 'bundles') diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java index 029f856a6..3faadc65d 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009, 2016 IBM Corporation and others. + * Copyright (c) 2009, 2018 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -10,13 +10,28 @@ *******************************************************************************/ package org.eclipse.osgi.tests.bundles; -import java.io.*; -import java.net.*; -import java.util.*; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URISyntaxException; +import java.net.URL; +import java.net.URLEncoder; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import junit.framework.Test; import junit.framework.TestSuite; +import org.eclipse.osgi.service.urlconversion.URLConverter; import org.eclipse.osgi.tests.OSGiTestsActivator; -import org.osgi.framework.*; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleException; +import org.osgi.framework.Constants; +import org.osgi.framework.InvalidSyntaxException; +import org.osgi.framework.ServiceRegistration; import org.osgi.framework.hooks.bundle.CollisionHook; public class BundleInstallUpdateTests extends AbstractBundleTests { @@ -367,4 +382,49 @@ public class BundleInstallUpdateTests extends AbstractBundleTests { is = encodedURL.openStream(); is.close(); } + + public void testEscapeZipRoot() throws IOException, BundleException, InvalidSyntaxException, URISyntaxException { + String entry1 = "../../escapedZipRoot1.txt"; + String entry2 = "dir1/../../../escapedZipRoot2.txt"; + String cp1 = "../../cp.jar"; + File bundlesDirectory = OSGiTestsActivator.getContext().getDataFile(getName()); + bundlesDirectory.mkdirs(); + Map headers = new HashMap(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, getName()); + headers.put(Constants.BUNDLE_CLASSPATH, "., ../../cp.jar"); + Map entries = new HashMap<>(); + entries.put(entry1, "value"); + entries.put(entry2, "value"); + entries.put(cp1, "value"); + + File testBundleJarFile = SystemBundleTests.createBundle(bundlesDirectory, getName(), headers, entries); + Bundle testBundle = getContext().installBundle(getName(), new FileInputStream(testBundleJarFile)); + testBundle.start(); + try { + testBundle.loadClass("does.not.exist.Test"); + } catch (ClassNotFoundException e) { + // expected + } + URLConverter bundleURLConverter = getContext().getService(getContext().getServiceReferences(URLConverter.class, "(protocol=bundleentry)").iterator().next()); + + URL dir1 = bundleURLConverter.toFileURL(testBundle.getEntry("dir1/")); + File dir1File = new File(dir1.toExternalForm().substring(5)); + + File dir1EscapedFile2 = new File(dir1File, entry2.substring("dir1".length())); + assertFalse("File escaped zip root: " + dir1EscapedFile2.getCanonicalPath(), dir1EscapedFile2.exists()); + + URL root = bundleURLConverter.toFileURL(testBundle.getEntry("/")); + File rootFile = new File(root.toExternalForm().substring(5)); + + File rootEscapedFile1 = new File(rootFile, entry1); + assertFalse("File escaped zip root: " + rootEscapedFile1.getCanonicalPath(), rootEscapedFile1.exists()); + + File rootEscapedFile2 = new File(rootFile, entry2); + assertFalse("File escaped zip root: " + rootEscapedFile2.getCanonicalPath(), rootEscapedFile2.exists()); + + File rootEscapedFile3 = new File(rootFile, cp1); + assertFalse("File escaped zip root: " + rootEscapedFile3.getCanonicalPath(), rootEscapedFile3.exists()); + + } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java index e4ebfd822..e5ddf7161 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2017 IBM Corporation and others. + * Copyright (c) 2012, 2018 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -41,6 +41,7 @@ import org.eclipse.osgi.internal.framework.EquinoxContainer; import org.eclipse.osgi.internal.hookregistry.StorageHookFactory; import org.eclipse.osgi.internal.hookregistry.StorageHookFactory.StorageHook; import org.eclipse.osgi.internal.messages.Msg; +import org.eclipse.osgi.storage.Storage.StorageException; import org.eclipse.osgi.storage.bundlefile.BundleEntry; import org.eclipse.osgi.storage.bundlefile.BundleFile; import org.eclipse.osgi.storage.url.BundleResourceHandler; @@ -308,15 +309,31 @@ public final class BundleInfo { * not exist if the content has not previously been stored. * @param path the path to the content to extract from the generation * @return a file object where content of the specified path may be stored. + * @throws StorageException if the path will escape the persistent storage of the generation */ public File getExtractFile(String path) { - StringBuilder builder = new StringBuilder(); - builder.append(getBundleId()).append('/').append(getGenerationId()); - if (path.length() > 0 && path.charAt(0) != '/') { - builder.append('/'); + return getExtractFile(null, path); + } + + /** + * Gets called by BundleFile during {@link BundleFile#getFile(String, boolean)}. This method + * will allocate a File object where content of the specified path may be + * stored for this generation. The returned File object may + * not exist if the content has not previously been stored. + * @param path the path to the content to extract from the generation + * @param base the base path that is prepended to the path, may be null + * @return a file object where content of the specified path may be stored. + * @throws StorageException if the path will escape the persistent storage of + * the generation starting at the specified base + */ + public File getExtractFile(String base, String path) { + StringBuilder baseBuilder = new StringBuilder(); + baseBuilder.append(getBundleId()).append('/').append(getGenerationId()); + if (base != null) { + baseBuilder.append('/').append(base); } - builder.append(path); - return getStorage().getFile(builder.toString(), true); + + return getStorage().getFile(baseBuilder.toString(), path, true); } public void storeContent(File destination, InputStream in, boolean nativeCode) throws IOException { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java index 9b89de67b..578581f19 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java @@ -110,6 +110,27 @@ import org.osgi.resource.Namespace; import org.osgi.resource.Requirement; public class Storage { + public static class StorageException extends RuntimeException { + private static final long serialVersionUID = 1L; + + public StorageException() { + super(); + } + + public StorageException(String message, Throwable cause) { + super(message, cause); + } + + public StorageException(String message) { + super(message); + } + + public StorageException(Throwable cause) { + super(cause); + } + + } + public static final int VERSION = 4; private static final int MR_JAR_VERSION = 4; private static final int LOWEST_VERSION_SUPPORTED = 3; @@ -931,15 +952,19 @@ public class Storage { return bundleID + "/" + generationID + "/" + BUNDLE_FILE_NAME; //$NON-NLS-1$ //$NON-NLS-2$ } - public File getFile(String path, boolean checkParent) { + public File getFile(String path, boolean checkParent) throws StorageException { + return getFile(null, path, checkParent); + } + + public File getFile(String base, String path, boolean checkParent) throws StorageException { // first check the child location - File childPath = new File(childRoot, path); + File childPath = getFile(childRoot, base, path); // now check the parent if (checkParent && parentRoot != null) { if (childPath.exists()) { return childPath; } - File parentPath = new File(parentRoot, path); + File parentPath = getFile(parentRoot, base, path); if (parentPath.exists()) { // only use the parent file only if it exists; return parentPath; @@ -949,6 +974,25 @@ public class Storage { return childPath; } + private static File getFile(File root, String base, String path) { + if (base != null) { + // if base is not null then move root to include the base + root = new File(root, base); + } + File result = new File(root, path); + + try { + String resultCanonical = result.getCanonicalPath(); + String rootCanonical = root.getCanonicalPath(); + if (!resultCanonical.startsWith(rootCanonical + File.separator) && !resultCanonical.equals(rootCanonical)) { + throw new StorageException("Invalid path: " + path); //$NON-NLS-1$ + } + } catch (IOException e) { + throw new StorageException("Invalid path: " + path, e); //$NON-NLS-1$ + } + return result; + } + private File stageContent(final InputStream in, final URL sourceURL) throws BundleException { if (System.getSecurityManager() == null) return stageContent0(in, sourceURL); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java index 915f5a47d..03bee5a59 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java @@ -31,6 +31,7 @@ import org.eclipse.osgi.internal.debug.Debug; import org.eclipse.osgi.internal.framework.EquinoxContainer; import org.eclipse.osgi.internal.messages.Msg; import org.eclipse.osgi.storage.BundleInfo; +import org.eclipse.osgi.storage.Storage.StorageException; import org.eclipse.osgi.util.NLS; /** @@ -190,13 +191,7 @@ public class ZipBundleFile extends BundleFile { private File getExtractFile(String entryName) { if (generation == null) return null; - String path = ".cp"; /* put all these entries in this subdir *///$NON-NLS-1$ - String name = entryName.replace('/', File.separatorChar); - if ((name.length() > 1) && (name.charAt(0) == File.separatorChar)) /* if name has a leading slash */ - path = path.concat(name); - else - path = path + File.separator + name; - return generation.getExtractFile(path); + return generation.getExtractFile(".cp", entryName); //$NON-NLS-1$ } public File getFile(String entry, boolean nativeCode) { @@ -241,6 +236,10 @@ public class ZipBundleFile extends BundleFile { if (debug.DEBUG_BUNDLE_FILE) Debug.printStackTrace(e); generation.getBundleInfo().getStorage().getLogServices().log(EquinoxContainer.NAME, FrameworkLogEntry.ERROR, "Unable to extract content: " + generation.getRevision() + ": " + entry, e); //$NON-NLS-1$ //$NON-NLS-2$ + } catch (StorageException e) { + if (debug.DEBUG_BUNDLE_FILE) + Debug.printStackTrace(e); + generation.getBundleInfo().getStorage().getLogServices().log(EquinoxContainer.NAME, FrameworkLogEntry.ERROR, "Unable to extract content: " + generation.getRevision() + ": " + entry, e); //$NON-NLS-1$ //$NON-NLS-2$ } } finally { openLock.unlock(); -- cgit v1.2.3