commit 78cf73473dda5ceee3eecda5169621f36b93c3db
Author: Jiri Vanek <jvanek@redhat.com>
Date: Tue Jun 18 15:37:47 2019 +0200
Fixed bug when relative path (..) could leak up (even out of cache)
--- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java
+++ a/netx/net/sourceforge/jnlp/cache/CacheUtil.java
@@ -696,46 +696,68 @@
path.append(location.getPort());
path.append(File.separatorChar);
}
- path.append(location.getPath().replace('/', File.separatorChar));
- if (location.getQuery() != null && !location.getQuery().trim().isEmpty()) {
- path.append(".").append(location.getQuery());
- }
-
- File candidate = new File(FileUtils.sanitizePath(path.toString()));
- if (candidate.getName().length() > 255) {
- /**
- * When filename is longer then 255 chars, then then various
- * filesytems have issues to save it. By saving the file by its
- * summ, we are trying to prevent collision of two files differs in
- * suffixes (general suffix of name, not only 'filetype suffix')
- * only. It is also preventing bug when truncate (files with 1000
- * chars hash in query) cuts to much.
- */
+ String locationPath = location.getPath().replace('/', File.separatorChar);
+ if (locationPath.contains("..")){
try {
- MessageDigest md = MessageDigest.getInstance("SHA-256");
- byte[] sum = md.digest(candidate.getName().getBytes(StandardCharsets.UTF_8));
- //convert the byte to hex format method 2
- StringBuilder hexString = new StringBuilder();
- for (int i = 0; i < sum.length; i++) {
- hexString.append(Integer.toHexString(0xFF & sum[i]));
- }
- String extension = "";
- int i = candidate.getName().lastIndexOf('.');
- if (i > 0) {
- extension = candidate.getName().substring(i);//contains dot
- }
- if (extension.length() < 10 && extension.length() > 1) {
- hexString.append(extension);
- }
- candidate = new File(candidate.getParentFile(), hexString.toString());
+ /**
+ * if path contains .. then it can harm lcoal system
+ * So without mercy, hash it
+ */
+ String hexed = hex(new File(locationPath).getName(), locationPath);
+ return new File(path.toString(), hexed.toString());
} catch (NoSuchAlgorithmException ex) {
- // should not occure, cite from javadoc:
- // every java iomplementation should support
+ // should not occur, cite from javadoc:
+ // every java implementation should support
// MD5 SHA-1 SHA-256
throw new RuntimeException(ex);
}
- }
- return candidate;
+ } else {
+ path.append(locationPath);
+ if (location.getQuery() != null && !location.getQuery().trim().isEmpty()) {
+ path.append(".").append(location.getQuery());
+ }
+
+ File candidate = new File(FileUtils.sanitizePath(path.toString()));
+ try {
+ if (candidate.getName().length() > 255) {
+ /**
+ * When filename is longer then 255 chars, then then various
+ * filesystems have issues to save it. By saving the file by its
+ * sum, we are trying to prevent collision of two files differs in
+ * suffixes (general suffix of name, not only 'filetype suffix')
+ * only. It is also preventing bug when truncate (files with 1000
+ * chars hash in query) cuts to much.
+ */
+ String hexed = hex(candidate.getName(), candidate.getName());
+ candidate = new File(candidate.getParentFile(), hexed.toString());
+ }
+ } catch (NoSuchAlgorithmException ex) {
+ // should not occur, cite from javadoc:
+ // every java implementation should support
+ // MD5 SHA-1 SHA-256
+ throw new RuntimeException(ex);
+ }
+ return candidate;
+ }
+ }
+
+ private static String hex(String origName, String candidate) throws NoSuchAlgorithmException {
+ MessageDigest md = MessageDigest.getInstance("SHA-256");
+ byte[] sum = md.digest(candidate.getBytes(StandardCharsets.UTF_8));
+ //convert the byte to hex format method 2
+ StringBuilder hexString = new StringBuilder();
+ for (int i = 0; i < sum.length; i++) {
+ hexString.append(Integer.toHexString(0xFF & sum[i]));
+ }
+ String extension = "";
+ int i = origName.lastIndexOf('.');
+ if (i > 0) {
+ extension = origName.substring(i);//contains dot
+ }
+ if (extension.length() < 10 && extension.length() > 1) {
+ hexString.append(extension);
+ }
+ return hexString.toString();
}
/**
diff --git a/netx/net/sourceforge/jnlp/util/FileUtils.java b/netx/net/sourceforge/jnlp/util/FileUtils.java
index 89216375..a5356e08 100644
--- a/netx/net/sourceforge/jnlp/util/FileUtils.java
+++ b/netx/net/sourceforge/jnlp/util/FileUtils.java
@@ -183,6 +183,13 @@
*/
public static void createParentDir(File f, String eMsg) throws IOException {
File parent = f.getParentFile();
+ // warning, linux and windows behave differently. Below snippet will pass on win(security hole), fail on linux
+ // warning mkdir is canonicaling, but exists/isDirectory is not. So where mkdirs return true, and really creates dir, isDirectory can still return false
+ // can be seen on this example
+ // mkdirs /a/b/../c
+ // where b do not exists will lead creation of /a/c
+ // but exists on /a/b/../c is false on linux even afterwards
+ // without hexing of .. paths,
if (!parent.isDirectory() && !parent.mkdirs()) {
throw new IOException(R("RCantCreateDir",
eMsg == null ? parent : eMsg));
diff --git a/tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java
index 6422246b..0d2d9811 100644
--- a/tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java
@@ -88,6 +88,53 @@ public class CacheUtilTest {
final File expected = new File("/tmp/https/example.com/5050/applet/e4f3cf11f86f5aa33f424bc3efe3df7a9d20837a6f1a5bbbc60c1f57f3780a4");
Assert.assertEquals(expected, CacheUtil.urlToPath(u, "/tmp"));
}
+
+ @Test
+ public void tesPathUpNoGoBasic() throws Exception {
+ final URL u = new URL("https://example.com/applet/../my.jar");
+ final File expected = new File("/tmp/https/example.com/abca4723622ed60db3dea12cbe2402622a74f7a49b73e23b55988e4eee5ded.jar");
+ File r = CacheUtil.urlToPath(u, "/tmp/");
+ Assert.assertEquals(expected, r);
+ }
+
+ @Test
+ public void tesPathUpNoGoBasicLong() throws Exception {
+ final URL u = new URL("https://example.com/applet/../my.jar.q_SlNFU1NJT05JRD02OUY1ODVCNkJBOTM1NThCQjdBMTA5RkQyNDZEQjEwRi5wcm9kX3RwdG9tY2F0MjE1X2p2bTsgRW50cnVzdFRydWVQYXNzUmVkaXJlY3RVcmw9Imh0dHBzOi8vZWZzLnVzcHRvLmdvdi9FRlNXZWJVSVJlZ2lzdGVyZWQvRUZTV2ViUmVnaXN0ZXJlZCI7IFRDUFJPRFBQQUlSc2Vzc2lvbj02MjIxMjk0MTguMjA0ODAuMDAwMA\"");
+ final File expected = new File("/tmp/https/example.com/ec97413e3f6eee8215ecc8375478cc1ae5f44f18241b9375361d5dfcd7b0ec");
+ File r = CacheUtil.urlToPath(u, "/tmp/");
+ Assert.assertEquals(expected, r);
+ }
+
+ @Test
+ public void tesPathUpNoGoBasic2() throws Exception {
+ final URL u = new URL("https://example.com/../my.jar");
+ final File expected = new File("/tmp/https/example.com/eb1a56bed34523dbe7ad84d893ebc31a8bbbba9ce3f370e42741b6a5f067c140.jar");
+ File r = CacheUtil.urlToPath(u, "/tmp/");
+ Assert.assertEquals(expected, r);
+ }
+
+ @Test
+ public void tesPathUpNoGoBasicEvil() throws Exception {
+ final URL u = new URL("https://example.com/../../my.jar");
+ final File expected = new File("/tmp/https/example.com/db464f11d68af73e37eefaef674517b6be23f0e4a5738aaee774ecf5b58f1bfc.jar");
+ File r = CacheUtil.urlToPath(u, "/tmp/");
+ Assert.assertEquals(expected, r);
+ }
+
+ @Test
+ public void tesPathUpNoGoBasicEvil2() throws Exception {
+ final URL u = new URL("https://example.com:99/../../../my.jar");
+ final File expected = new File("/tmp/https/example.com/99/95401524c345e0d554d4d77330e86c98a77b9bb58a0f93094204df446b356.jar");
+ File r = CacheUtil.urlToPath(u, "/tmp/");
+ Assert.assertEquals(expected, r);
+ }
+ @Test
+ public void tesPathUpNoGoBasicEvilest() throws Exception {
+ final URL u = new URL("https://example2.com/something/../../../../../../../../../../../my.jar");
+ final File expected = new File("/tmp/https/example2.com/a8df64388f5b84d5f635e4d6dea5f4d2f692ae5381f8ec6736825ff8d6ff2c0.jar");
+ File r = CacheUtil.urlToPath(u, "/tmp/");
+ Assert.assertEquals(expected, r);
+ }
@Test
diff --git a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPClassLoaderTest.java b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPClassLoaderTest.java
index 100d9150..7580d23b 100644
--- a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPClassLoaderTest.java
+++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPClassLoaderTest.java
@@ -43,6 +43,8 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.InputStream;
+import java.net.URL;
+import java.nio.charset.Charset;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.List;
@@ -55,6 +57,12 @@
import net.sourceforge.jnlp.browsertesting.browsers.firefox.FirefoxProfilesOperator;
import net.sourceforge.jnlp.cache.UpdatePolicy;
import net.sourceforge.jnlp.config.DeploymentConfiguration;
+import net.sourceforge.jnlp.config.PathsAndFiles;
+import net.sourceforge.jnlp.JNLPFile;
+import net.sourceforge.jnlp.ServerAccess;
+import net.sourceforge.jnlp.ServerLauncher;
+import net.sourceforge.jnlp.util.StreamUtils;
+import net.sourceforge.jnlp.cache.CacheUtil;
import net.sourceforge.jnlp.mock.DummyJNLPFileWithJar;
import net.sourceforge.jnlp.security.appletextendedsecurity.AppletSecurityLevel;
import net.sourceforge.jnlp.security.appletextendedsecurity.AppletStartupSecuritySettings;
@@ -65,6 +73,7 @@
import org.junit.BeforeClass;
import org.junit.Test;
+import org.junit.Ignore;
public class JNLPClassLoaderTest extends NoStdOutErrTest {
@@ -138,7 +147,8 @@
File tempDirectory = FileTestUtils.createTempDirectory();
File jarLocation = new File(tempDirectory, "test.jar");
- /* Test with main-class in manifest */ {
+ /* Test with main-class in manifest */
+ {
Manifest manifest = new Manifest();
manifest.getMainAttributes().put(Attributes.Name.MAIN_CLASS, "DummyClass");
FileTestUtils.createJarWithContents(jarLocation, manifest);
@@ -156,8 +166,10 @@
}
@Test
+ @Ignore
public void getMainClassNameTestEmpty() throws Exception {
- /* Test with-out any main-class specified */ {
+ /* Test with-out any main-class specified */
+ {
File tempDirectory = FileTestUtils.createTempDirectory();
File jarLocation = new File(tempDirectory, "test.jar");
FileTestUtils.createJarWithContents(jarLocation /* No contents */);
@@ -363,4 +375,57 @@
}
}
+
+ @Test
+ public void testRelativePathInUrl() throws Exception {
+ CacheUtil.clearCache();
+ int port = ServerAccess.findFreePort();
+ File dir = FileTestUtils.createTempDirectory();
+ dir.deleteOnExit();
+ dir = new File(dir,"base");
+ dir.mkdir();
+ File jar = new File(dir,"j1.jar");
+ File jnlp = new File(dir+"/a/b/up.jnlp");
+ jnlp.getParentFile().mkdirs();
+ InputStream is = ClassLoader.getSystemClassLoader().getResourceAsStream("net/sourceforge/jnlp/runtime/up.jnlp");
+ String jnlpString = StreamUtils.readStreamAsString(is, true, "utf-8");
+ is.close();
+ jnlpString = jnlpString.replaceAll("8080", ""+port);
+ is = ClassLoader.getSystemClassLoader().getResourceAsStream("net/sourceforge/jnlp/runtime/j1.jar");
+ StreamUtils.copyStream(is, new FileOutputStream(jar));
+ Files.write(jnlp.toPath(),jnlpString.getBytes("utf-8"));
+ ServerLauncher as = ServerAccess.getIndependentInstance(jnlp.getParent(), port);
+ boolean verifyBackup = JNLPRuntime.isVerifying();
+ boolean trustBackup= JNLPRuntime.isTrustAll();
+ boolean securityBAckup= JNLPRuntime.isSecurityEnabled();
+ boolean verbose= JNLPRuntime.isDebug();
+ JNLPRuntime.setVerify(false);
+ JNLPRuntime.setTrustAll(true);
+ JNLPRuntime.setSecurityEnabled(false);
+ JNLPRuntime.setDebug(true);
+ try {
+ final JNLPFile jnlpFile1 = new JNLPFile(new URL("http://localhost:" + port + "/up.jnlp"));
+ final JNLPClassLoader classLoader1 = new JNLPClassLoader(jnlpFile1, UpdatePolicy.ALWAYS) {
+ @Override
+ protected void activateJars(List<JARDesc> jars) {
+ super.activateJars(jars);
+ }
+
+ };
+ InputStream is1 = classLoader1.getResourceAsStream("Hello1.class");
+ is1.close();
+ is1 = classLoader1.getResourceAsStream("META-INF/MANIFEST.MF");
+ is1.close();
+ Assert.assertTrue(new File(PathsAndFiles.CACHE_DIR.getFullPath()+"/0/http/localhost/"+port+"/up.jnlp").exists());
+ Assert.assertTrue(new File(PathsAndFiles.CACHE_DIR.getFullPath()+"/1/http/localhost/"+port+"/f812acb32c857fd916c842e2bf4fb32b9c3837ef63922b167a7e163305058b7.jar").exists());
+ } finally {
+ JNLPRuntime.setVerify(verifyBackup);
+ JNLPRuntime.setTrustAll(trustBackup);
+ JNLPRuntime.setSecurityEnabled(securityBAckup);
+ JNLPRuntime.setDebug(verbose);
+ as.stop();
+ }
+
+ }
+
}
diff --git a/tests/netx/unit/net/sourceforge/jnlp/runtime/up.jnlp b/tests/netx/unit/net/sourceforge/jnlp/runtime/up.jnlp
new file mode 100644
index 00000000..b22fdfb7
--- /dev/null
+++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/up.jnlp
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<jnlp spec="6.0+" codebase=".">
+
+<information><title>1965</title><vendor>Nemzeti Ado- es Vamhivatal</vendor><offline-allowed/></information>
+
+
+<resources>
+ <j2se href="http://java.sun.com/products/autodl/j2se" version="1.8+" />
+<!-- absolute url is a must -->
+ <jar href="http://localhost:8080/../../../base/j1.jar" version="2.0"/>
+</resources>
+
+<application-desc main-class="Hello1" />
+
+</jnlp>