Bug 395274 - Equinox returns valid bundle entries for invalid paths

3 files changed, 123 insertions, 11 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java
index b97c1a0fb..ff18bcb26 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2011 IBM Corporation and others.
+ * Copyright (c) 2010, 2013 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
@@ -15,6 +15,7 @@ import java.util.Enumeration;
 import junit.framework.Test;
 import junit.framework.TestSuite;
 import org.eclipse.core.tests.harness.CoreTest;
+import org.eclipse.osgi.framework.internal.core.FrameworkProperties;
 import org.eclipse.osgi.tests.OSGiTestsActivator;
 import org.osgi.framework.*;
 
@@ -65,6 +66,31 @@ public class BundleResourceTests extends CoreTest {
 		assertNotNull("Did not find resource!", paths);
 	}
 
+	public void testBug395274() throws Exception {
+		String original = FrameworkProperties.setProperty("osgi.strictBundleEntryPath", "true");
+		try {
+			Bundle bundle = installer.installBundle("test"); //$NON-NLS-1$
+			URL path = bundle.getEntry("META-INF./MANIFEST.MF");
+			assertNull("found resource!", path);
+			path = bundle.getEntry("META-INF/MANIFEST.MF");
+			assertNotNull("Did not find resource!", path);
+			path = bundle.getEntry("folder/file1.TXT");
+			assertNull("found resource!", path);
+			path = bundle.getEntry("folder/file1.txt");
+			assertNotNull("Did not find resource!", path);
+			checkEntries(bundle, "/./file1.txt", 1);
+			checkEntries(bundle, "//file1.txt", 1);
+			checkEntries(bundle, "/", 1);
+			checkEntries(bundle, "/.", 1);
+		} finally {
+			if (original != null) {
+				FrameworkProperties.setProperty("osgi.strictBundleEntryPath", original);
+			} else {
+				FrameworkProperties.clearProperty("osgi.strictBundleEntryPath");
+			}
+		}
+	}
+
 	public void testBug328795() throws BundleException {
 		Bundle bundle = installer.installBundle("test"); //$NON-NLS-1$
 		checkEntries(bundle, "notFound\\", 0); // this results in invalid syntax exception which is logged because of trailing escape
diff --git a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java
index 64c3f7191..295a8667e 100644
--- a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java
+++ b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2010 IBM Corporation and others.
+ * Copyright (c) 2005, 2013 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
@@ -24,6 +24,15 @@ import org.eclipse.osgi.util.NLS;
  */
 public class DirBundleFile extends BundleFile {
 
+	private static final String POINTER_SAME_DIRECTORY_1 = "/.";//$NON-NLS-1$
+	private static final String POINTER_SAME_DIRECTORY_2 = "//";//$NON-NLS-1$
+	private static final String POINTER_UPPER_DIRECTORY = "..";//$NON-NLS-1$
+
+	private static final String PROPERTY_STRICT_BUNDLE_ENTRY_PATH = "osgi.strictBundleEntryPath";//$NON-NLS-1$
+	private static final String PROPERTY_STRICT_BUNDLE_ENTRY_PATH_DEFAULT_VALUE = "false";//$NON-NLS-1$
+
+	private final boolean enableStrictBundleEntryPath;
+
 	/**
 	 * Constructs a DirBundleFile
 	 * @param basefile the base file
@@ -34,23 +43,62 @@ public class DirBundleFile extends BundleFile {
 		if (!BundleFile.secureAction.exists(basefile) || !BundleFile.secureAction.isDirectory(basefile)) {
 			throw new IOException(NLS.bind(AdaptorMsg.ADAPTOR_DIRECTORY_EXCEPTION, basefile));
 		}
+		this.enableStrictBundleEntryPath = Boolean.parseBoolean(BundleFile.secureAction.getProperty(PROPERTY_STRICT_BUNDLE_ENTRY_PATH, PROPERTY_STRICT_BUNDLE_ENTRY_PATH_DEFAULT_VALUE));
 	}
 
 	public File getFile(String path, boolean nativeCode) {
-		boolean checkInBundle = path != null && path.indexOf("..") >= 0; //$NON-NLS-1$
-		File file = new File(basefile, path);
+		final boolean checkInBundle = path != null && path.indexOf(POINTER_UPPER_DIRECTORY) >= 0;
+		File file = new File(this.basefile, path);
 		if (!BundleFile.secureAction.exists(file)) {
 			return null;
 		}
-		// must do an extra check to make sure file is within the bundle (bug 320546)
-		if (checkInBundle) {
-			try {
-				if (!BundleFile.secureAction.getCanonicalPath(file).startsWith(BundleFile.secureAction.getCanonicalPath(basefile)))
+
+		if (!enableStrictBundleEntryPath) {
+			if (checkInBundle) {
+				try {
+					if (!BundleFile.secureAction.getCanonicalPath(file).startsWith(BundleFile.secureAction.getCanonicalPath(basefile)))
+						return null;
+				} catch (IOException e) {
+					return null;
+				}
+			}
+			return file;
+		}
+		boolean normalize = false;
+		boolean isBundleRoot = false;
+		if (path != null) {
+			isBundleRoot = path.equals("/");//$NON-NLS-1$
+			if (!isBundleRoot) {
+				normalize = checkInBundle || path.indexOf(POINTER_SAME_DIRECTORY_1) >= 0 || path.indexOf(POINTER_SAME_DIRECTORY_2) >= 0;
+			}
+		}
+		File canonicalFile;
+		try {
+			canonicalFile = BundleFile.secureAction.getCanonicalFile(file);
+			if (!isBundleRoot) {
+				File absoluteFile = BundleFile.secureAction.getAbsoluteFile(file);
+				String canonicalPath;
+				String absolutePath;
+				if (normalize) {
+					canonicalPath = canonicalFile.toURI().getPath();
+					absolutePath = absoluteFile.toURI().normalize().getPath();
+				} else {
+					canonicalPath = canonicalFile.getPath();
+					absolutePath = absoluteFile.getPath();
+				}
+				if (!canonicalPath.equals(absolutePath)) {
+					return null;
+				}
+			}
+			// must do an extra check to make sure file is within the bundle (bug 320546)
+			if (checkInBundle) {
+				if (!canonicalFile.getPath().startsWith(BundleFile.secureAction.getCanonicalPath(basefile)))
 					return null;
-			} catch (IOException e) {
-				return null;
 			}
+		} catch (IOException e) {
+			return null;
 		}
+
 		return file;
 	}
 
diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java
index 2e654b16c..5fb4cef0d 100644
--- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java
+++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2010 IBM Corporation and others.
+ * Copyright (c) 2003, 2013 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
@@ -198,6 +198,44 @@ public class SecureAction {
 	}
 
 	/**
+	 * Returns the absolute file.  Same as calling
+	 * file.getAbsoluteFile().
+	 * @param file a file object
+	 * @return the absolute file.
+	 */
+	public File getAbsoluteFile(final File file) {
+		if (System.getSecurityManager() == null)
+			return file.getAbsoluteFile();
+		return AccessController.doPrivileged(new PrivilegedAction<File>() {
+			public File run() {
+				return file.getAbsoluteFile();
+			}
+		}, controlContext);
+	}
+
+	/**
+	 * Returns the canonical file.  Same as calling
+	 * file.getCanonicalFile().
+	 * @param file a file object
+	 * @return the canonical file.
+	 */
+	public File getCanonicalFile(final File file) throws IOException {
+		if (System.getSecurityManager() == null)
+			return file.getCanonicalFile();
+		try {
+			return AccessController.doPrivileged(new PrivilegedExceptionAction<File>() {
+				public File run() throws IOException {
+					return file.getCanonicalFile();
+				}
+			}, controlContext);
+		} catch (PrivilegedActionException e) {
+			if (e.getException() instanceof IOException)
+				throw (IOException) e.getException();
+			throw (RuntimeException) e.getException();
+		}
+	}
+
+	/**
 	 * Returns true if a file exists, otherwise false is returned.  Same as calling
 	 * file.exists().
 	 * @param file a file object