aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2018-06-13 10:02:54 -0400
committerThomas Watson2018-06-13 12:28:13 -0400
commit2c660df89f34884d8fb27eebbaa12df68cc3f2e8 (patch)
tree30b30effb05dc37a88bcedd101657cd68002d2d8
parent2d823fee68317809d7d8bbad4cf5a2b5aacec19b (diff)
downloadrt.equinox.framework-2c660df89f34884d8fb27eebbaa12df68cc3f2e8.zip
rt.equinox.framework-2c660df89f34884d8fb27eebbaa12df68cc3f2e8.tar.gz
rt.equinox.framework-2c660df89f34884d8fb27eebbaa12df68cc3f2e8.tar.xz
Bug 535822 - Check for directory traversal entry paths
Change-Id: Ic989745f1115d3c23866957519f1d9fc8f315640 Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java70
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java31
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java50
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java13
4 files changed, 142 insertions, 22 deletions
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 029f856..3faadc6 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<String, String> headers = new HashMap<String, String>();
+ headers.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ headers.put(Constants.BUNDLE_SYMBOLICNAME, getName());
+ headers.put(Constants.BUNDLE_CLASSPATH, "., ../../cp.jar");
+ Map<String, String> 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 e4ebfd8..e5ddf71 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 9b89de6..578581f 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 915f5a4..03bee5a 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();