Author: pauls Date: Fri Jun 15 14:02:50 2018 New Revision: 1833597 URL: http://svn.apache.org/viewvc?rev=1833597&view=rev Log: FELIX-5870: Don't allow relative path instructions in bundleclasspath to prevent access outside the bundle cache.
Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java felix/trunk/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java?rev=1833597&r1=1833596&r2=1833597&view=diff ============================================================================== --- felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java (original) +++ felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java Fri Jun 15 14:02:50 2018 @@ -29,12 +29,8 @@ import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Enumeration; import java.util.HashMap; -import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Properties; @@ -183,6 +179,14 @@ public class DirectoryContent implements // entries are relative to the root of the bundle. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; + if (entryName.trim().startsWith(".." + File.separatorChar) || + entryName.contains(File.separator + ".." + File.separatorChar) || + entryName.trim().endsWith(File.separator + "..") || + entryName.trim().equals("..")) + { + return null; + } + // Any embedded JAR files will be extracted to the embedded directory. File embedDir = new File(m_rootDir, m_dir.getName() + EMBEDDED_DIRECTORY); @@ -202,18 +206,7 @@ public class DirectoryContent implements (entryName.lastIndexOf('/') >= 0) ? entryName.substring(0, entryName.lastIndexOf('/')) : entryName); - synchronized (m_revisionLock) - { - if (!BundleCache.getSecureAction().fileExists(extractDir)) - { - if (!BundleCache.getSecureAction().mkdirs(extractDir)) - { - m_logger.log( - Logger.LOG_ERROR, - "Unable to extract embedded directory."); - } - } - } + return new JarContent( m_logger, m_configMap, m_zipFactory, m_revisionLock, extractDir, file, null); @@ -233,6 +226,14 @@ public class DirectoryContent implements // entries are relative to the root of the bundle. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; + if (entryName.trim().startsWith(".." + File.separatorChar) || + entryName.contains(File.separator + ".." + File.separatorChar) || + entryName.trim().endsWith(File.separator + "..") || + entryName.trim().equals("..")) + { + return null; + } + // Any embedded native library files will be extracted to the lib directory. File libDir = new File(m_rootDir, m_dir.getName() + LIBRARY_DIRECTORY); Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java?rev=1833597&r1=1833596&r2=1833597&view=diff ============================================================================== --- felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java (original) +++ felix/trunk/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java Fri Jun 15 14:02:50 2018 @@ -208,6 +208,13 @@ public class JarContent implements Conte // Remove any leading slash. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; + if (entryName.trim().startsWith(".." + File.separatorChar) || + entryName.contains(File.separator + ".." + File.separatorChar) || + entryName.trim().endsWith(File.separator + "..") || + entryName.trim().equals("..")) + { + return null; + } // Any embedded JAR files will be extracted to the embedded directory. // Since embedded JAR file names may clash when extracting from multiple // embedded JAR files, the embedded directory is per embedded JAR file. @@ -220,52 +227,50 @@ public class JarContent implements Conte // directory in the bundle JAR file. Ignore any entries // that do not exist per the spec. ZipEntry ze = m_zipFile.getEntry(entryName); + if ((ze != null) && ze.isDirectory()) { - File extractDir = new File(embedDir, entryName); - - // Extracting an embedded directory file impacts all other existing - // contents for this revision, so we have to grab the revision - // lock first before trying to create a directory for an embedded - // directory to avoid a race condition. - synchronized (m_revisionLock) - { - if (!BundleCache.getSecureAction().fileExists(extractDir)) - { - if (!BundleCache.getSecureAction().mkdirs(extractDir)) - { - m_logger.log( - Logger.LOG_ERROR, - "Unable to extract embedded directory."); - } - } - } return new ContentDirectoryContent(this, entryName); } else if ((ze != null) && ze.getName().endsWith(".jar")) { File extractJar = new File(embedDir, entryName); - // Extracting the embedded JAR file impacts all other existing - // contents for this revision, so we have to grab the revision - // lock first before trying to extract the embedded JAR file - // to avoid a race condition. - synchronized (m_revisionLock) + try { - try - { - extractEmbeddedJar(entryName); - } - catch (Exception ex) + if (!BundleCache.getSecureAction().fileExists(extractJar)) { - m_logger.log( - Logger.LOG_ERROR, - "Unable to extract embedded JAR file.", ex); + // Extracting the embedded JAR file impacts all other existing + // contents for this revision, so we have to grab the revision + // lock first before trying to extract the embedded JAR file + // to avoid a race condition. + synchronized (m_revisionLock) + { + if (!BundleCache.getSecureAction().fileExists(extractJar)) + { + // Make sure that the embedded JAR's parent directory exists; + // it may be in a sub-directory. + File jarDir = extractJar.getParentFile(); + if (!BundleCache.getSecureAction().fileExists(jarDir) && !BundleCache.getSecureAction().mkdirs(jarDir)) + { + throw new IOException("Unable to create embedded JAR directory."); + } + + // Extract embedded JAR into its directory. + BundleCache.copyStreamToFile(m_zipFile.getInputStream(ze), extractJar); + } + } } + return new JarContent( + m_logger, m_configMap, m_zipFactory, m_revisionLock, + extractJar.getParentFile(), extractJar, null); + } + catch (Exception ex) + { + m_logger.log( + Logger.LOG_ERROR, + "Unable to extract embedded JAR file.", ex); } - return new JarContent( - m_logger, m_configMap, m_zipFactory, m_revisionLock, - extractJar.getParentFile(), extractJar, null); } // The entry could not be found, so return null. @@ -281,6 +286,14 @@ public class JarContent implements Conte // Remove any leading slash. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; + if (entryName.trim().startsWith(".." + File.separatorChar) || + entryName.contains(File.separator + ".." + File.separatorChar) || + entryName.trim().endsWith(File.separator + "..") || + entryName.trim().equals("..")) + { + return null; + } + // Any embedded native libraries will be extracted to the lib directory. // Since embedded library file names may clash when extracting from multiple // embedded JAR files, the embedded lib directory is per embedded JAR file. @@ -385,54 +398,6 @@ public class JarContent implements Conte return m_file; } - /** - * This method extracts an embedded JAR file from the bundle's - * JAR file. - * @param jarPath the path to the embedded JAR file inside the bundle JAR file. - **/ - private void extractEmbeddedJar(String jarPath) - throws Exception - { - // Remove leading slash if present. - jarPath = (jarPath.length() > 0) && (jarPath.charAt(0) == '/') - ? jarPath.substring(1) : jarPath; - - // Any embedded JAR files will be extracted to the embedded directory. - // Since embedded JAR file names may clash when extracting from multiple - // embedded JAR files, the embedded directory is per embedded JAR file. - File embedDir = new File(m_rootDir, m_file.getName() + EMBEDDED_DIRECTORY); - File jarFile = new File(embedDir, jarPath); - - if (!BundleCache.getSecureAction().fileExists(jarFile)) - { - // Make sure class path entry is a JAR file. - ZipEntry ze = m_zipFile.getEntry(jarPath); - if (ze == null) - { - return; - } - // If the zip entry is a directory, then ignore it since - // we don't need to extact it; otherwise, it points to an - // embedded JAR file, so extract it. - else if (!ze.isDirectory()) - { - // Make sure that the embedded JAR's parent directory exists; - // it may be in a sub-directory. - File jarDir = jarFile.getParentFile(); - if (!BundleCache.getSecureAction().fileExists(jarDir)) - { - if (!BundleCache.getSecureAction().mkdirs(jarDir)) - { - throw new IOException("Unable to create embedded JAR directory."); - } - } - - // Extract embedded JAR into its directory. - BundleCache.copyStreamToFile(m_zipFile.getInputStream(ze), jarFile); - } - } - } - private static class DevNullRunnable implements Runnable { private final InputStream m_in; Modified: felix/trunk/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java?rev=1833597&r1=1833596&r2=1833597&view=diff ============================================================================== --- felix/trunk/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java (original) +++ felix/trunk/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java Fri Jun 15 14:02:50 2018 @@ -96,6 +96,60 @@ public class BundleCacheTest extends Tes createJar(archiveFile, jarFile); } + public void testNoZipSlip() throws Exception + { + File bundle = new File(filesDir, "slip"); + Manifest manifest = new Manifest(); + manifest.getMainAttributes().putValue("Manifest-Version", "v1"); + manifest.getMainAttributes().putValue(Constants.BUNDLE_MANIFESTVERSION, "2"); + manifest.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, "slip"); + + JarOutputStream output = new JarOutputStream(new FileOutputStream(bundle),manifest); + + output.putNextEntry(new ZipEntry("../../bar.jar")); + + output.write(BundleCache.read(new FileInputStream(jarFile), jarFile.length())); + + output.closeEntry(); + + output.close(); + + BundleArchive archive = cache.create(1, 1, "slip", new FileInputStream(bundle)); + + testNoZipSlip(archive); + + archive = cache.create(1, 1, bundle.toURI().toURL().toString(), null); + + testNoZipSlip(archive); + + archive = cache.create(1, 1, "reference:" + bundle.toURI().toURL().toString(), null); + + testNoZipSlip(archive); + + File dir = new File(filesDir, "exploded"); + + dir.mkdirs(); + + File test = new File(dir, "../../bar.jar"); + test.createNewFile(); + test.deleteOnExit(); + + archive = cache.create(1, 1, "reference:" + dir.toURI().toURL().toString(), null); + + testNoZipSlip(archive); + } + + public void testNoZipSlip(BundleArchive archive) throws Exception + { + Content content = archive.getCurrentRevision().getContent().getEntryAsContent("../../bar.jar"); + + assertNull(content); + + String lib = archive.getCurrentRevision().getContent().getEntryAsNativeLibrary("../../bar.jar"); + + assertNull(lib); + } + public void testDirectoryReference() throws Exception { testBundle("reference:" + archiveFile.toURI().toURL(), null);