Merge pull request #70 from jenkinsci-cert/SECURITY-281

[SECURITY-281] Forbid /computer/(master)/config.xml

core/src/main/java/jenkins/model/Jenkins.java

@@ -298,6 +298,7 @@ import static hudson.init.InitMilestone.*;
 import hudson.util.LogTaskListener;
 import static java.util.logging.Level.*;
 import static javax.servlet.http.HttpServletResponse.*;
+import org.kohsuke.stapler.WebMethod;
 
 /**
  * Root object of the system.
@@ -3996,6 +3997,12 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
             Jenkins.getInstance().doConfigExecutorsSubmit(req, rsp);
         }
 
+        @WebMethod(name="config.xml")
+        @Override
+        public void doConfigDotXml(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+            throw HttpResponses.status(SC_BAD_REQUEST);
+        }
+
         @Override
         public boolean hasPermission(Permission permission) {
             // no one should be allowed to delete the master.

test/src/test/java/hudson/cli/GetNodeCommandTest.java

@@ -30,15 +30,14 @@ import static hudson.cli.CLICommandInvoker.Matcher.hasNoErrorOutput;
 import static hudson.cli.CLICommandInvoker.Matcher.succeeded;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.startsWith;
-import static org.hamcrest.text.IsEmptyString.isEmptyString;
 import hudson.model.Computer;
 import jenkins.model.Jenkins;
 
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 public class GetNodeCommandTest {
@@ -92,4 +91,18 @@ public class GetNodeCommandTest {
         assertThat(result, failedWith(-1));
         assertThat(result, hasNoStandardOutput());
     }
+
+    @Issue("SECURITY-281")
+    @Test
+    public void getNodeShouldFailForMaster() throws Exception {
+        CLICommandInvoker.Result result = command.authorizedTo(Computer.EXTENDED_READ, Jenkins.READ).invokeWithArgs("");
+        assertThat(result.stderr(), containsString("No such node ''"));
+        assertThat(result, failedWith(-1));
+        assertThat(result, hasNoStandardOutput());
+        result = command.authorizedTo(Computer.EXTENDED_READ, Jenkins.READ).invokeWithArgs("(master)");
+        assertThat(result.stderr(), containsString("No such node '(master)'"));
+        assertThat(result, failedWith(-1));
+        assertThat(result, hasNoStandardOutput());
+    }
+
 }

test/src/test/java/hudson/cli/UpdateNodeCommandTest.java

@@ -38,6 +38,7 @@ import jenkins.model.Jenkins;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 public class UpdateNodeCommandTest {
@@ -96,4 +97,18 @@ public class UpdateNodeCommandTest {
         assertThat(result, failedWith(-1));
         assertThat(result, hasNoStandardOutput());
     }
+
+    @Issue("SECURITY-281")
+    @Test
+    public void updateNodeShouldFailForMaster() throws Exception {
+        CLICommandInvoker.Result result = command.authorizedTo(Computer.CONFIGURE, Jenkins.READ).withStdin(Computer.class.getResourceAsStream("node.xml")).invokeWithArgs("");
+        assertThat(result.stderr(), containsString("No such node ''"));
+        assertThat(result, failedWith(-1));
+        assertThat(result, hasNoStandardOutput());
+        result = command.authorizedTo(Computer.EXTENDED_READ, Jenkins.READ).withStdin(Computer.class.getResourceAsStream("node.xml")).invokeWithArgs("(master)");
+        assertThat(result.stderr(), containsString("No such node '(master)'"));
+        assertThat(result, failedWith(-1));
+        assertThat(result, hasNoStandardOutput());
+    }
+
 }

test/src/test/java/hudson/model/NodeTest.java

@@ -23,6 +23,10 @@
  */
 package hudson.model;
 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.Page;
+import com.gargoylesoftware.htmlunit.WebRequestSettings;
 import hudson.EnvVars;
 import hudson.FilePath;
 import hudson.maven.MavenModuleSet;
@@ -41,6 +45,7 @@ import hudson.slaves.OfflineCause;
 import hudson.slaves.OfflineCause.ByCLI;
 import hudson.slaves.OfflineCause.UserCause;
 import hudson.util.TagCloud;
+import java.net.HttpURLConnection;
 
 import java.util.*;
 import java.util.concurrent.Callable;
@@ -400,6 +405,22 @@ public class NodeTest {
         assertThatCloudLabelDoesNotContain(cloud, "label1 label2", 0);
     }
 
+    @Issue("SECURITY-281")
+    @Test
+    public void masterComputerConfigDotXml() throws Exception {
+        JenkinsRule.WebClient wc = j.createWebClient();
+        wc.assertFails("computer/(master)/config.xml", HttpURLConnection.HTTP_BAD_REQUEST);
+        WebRequestSettings settings = new WebRequestSettings(wc.createCrumbedUrl("computer/(master)/config.xml"));
+        settings.setHttpMethod(HttpMethod.POST);
+        settings.setRequestBody("<hudson/>");
+        try {
+            Page page = wc.getPage(settings);
+            fail(page.getWebResponse().getContentAsString());
+        } catch (FailingHttpStatusCodeException x) {
+            assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, x.getStatusCode());
+        }
+    }
+
     /**
      * Assert that a tag cloud contains label name and weight.
      */