diff --git a/core/src/main/java/hudson/model/Slave.java b/core/src/main/java/hudson/model/Slave.java index d2c3bb871ba9f3a1f69bd4226e43a1086c72fc01..e5f5eebd976ee769d09e1a3321080c996b9dfc86 100644 --- a/core/src/main/java/hudson/model/Slave.java +++ b/core/src/main/java/hudson/model/Slave.java @@ -51,8 +51,10 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLConnection; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.TreeSet; import javax.servlet.ServletException; @@ -322,13 +324,14 @@ public abstract class Slave extends Node implements Serializable { public URL getURL() throws MalformedURLException { String name = fileName; - if (name.equals("hudson-cli.jar")) name="jenkins-cli.jar"; - // Prevent the sandbox escaping (SECURITY-195) - if (name.equals("..") || name.startsWith("../") || name.startsWith("..\\") || - name.replace('\\','/').contains("/../")) { - throw new MalformedURLException("The specified file path " + fileName + " contains '..'. " - + "The path is not allowed due to security reasons"); + // Prevent the access to war contents & prevent the folder escaping (SECURITY-195) + if (!ALLOWED_JNLPJARS_FILES.contains(name)) { + throw new MalformedURLException("The specified file path " + fileName + " is not allowed due to security reasons"); + } + + if (name.equals("hudson-cli.jar")) { + name="jenkins-cli.jar"; } URL res = Jenkins.getInstance().servletContext.getResource("/WEB-INF/" + name); @@ -505,4 +508,10 @@ public abstract class Slave extends Node implements Serializable { * Determines the workspace root file name for those who really really need the shortest possible path name. */ private static final String WORKSPACE_ROOT = System.getProperty(Slave.class.getName()+".workspaceRoot","workspace"); + + /** + * Provides a collection of file names, which are accessible via /jnlpJars link. + */ + private static final Set ALLOWED_JNLPJARS_FILES = new TreeSet + (Arrays.asList("slave.jar", "remoting.jar", "jenkins-cli.jar", "hudson-cli.jar")); } diff --git a/test/src/test/java/hudson/model/SlaveTest2.java b/test/src/test/java/hudson/model/SlaveTest2.java index d60ba017b07ae2c21c4f8e504e12f0f4fc7aa8cb..bc57f7ec99dc06d07c1e7a8d297703968d780431 100644 --- a/test/src/test/java/hudson/model/SlaveTest2.java +++ b/test/src/test/java/hudson/model/SlaveTest2.java @@ -52,9 +52,19 @@ public class SlaveTest2 { // Spot-check correct requests assertJnlpJarUrlIsAllowed(slave, "slave.jar"); + assertJnlpJarUrlIsAllowed(slave, "remoting.jar"); assertJnlpJarUrlIsAllowed(slave, "jenkins-cli.jar"); + assertJnlpJarUrlIsAllowed(slave, "hudson-cli.jar"); - // Go to the upper level + // Check that requests to other WEB-INF contents fail + assertJnlpJarUrlFails(slave, "web.xml"); + assertJnlpJarUrlFails(slave, "web.xml"); + assertJnlpJarUrlFails(slave, "classes/bundled-plugins.txt"); + assertJnlpJarUrlFails(slave, "classes/dependencies.txt"); + assertJnlpJarUrlFails(slave, "plugins/ant.hpi"); + assertJnlpJarUrlFails(slave, "nonexistentfolder/something.txt"); + + // Try various kinds of folder escaping (SECURITY-195) assertJnlpJarUrlFails(slave, "../"); assertJnlpJarUrlFails(slave, ".."); assertJnlpJarUrlFails(slave, "..\\"); @@ -89,7 +99,6 @@ public class SlaveTest2 { // Access from a Web client JenkinsRule.WebClient client = rule.createWebClient(); client.getPage(client.getContextPath() + "jnlpJars/" + URLEncoder.encode(url, "UTF-8")).getWebResponse().getContentAsString(); - client.getPage(jnlpJar.getURL()).getWebResponse().getContentAsString(); - + client.getPage(jnlpJar.getURL()).getWebResponse().getContentAsString(); } }