HuntersHunters
Sign inStart free
Security context
Medium· 4.3GHSA-pvwx-3jx5-24r2 CVE-2021-21639CWE-20Published May 24, 2022

Lack of type validation in agent related REST API in Jenkins

Research this vulnerability

Research is free — Hunters explains how the bug works, the root-cause code pattern, how the fix addresses it, and how to test whether a target is affected, in chat. Investigate & write exploit is a paid run — the engine reads the advisory and fix commits, then builds and validates a working proof-of-concept exploit with reproduction steps.

Affected versions

0 → fixed in 2.277.22.278 → fixed in 2.287

Details

Jenkins 2.286 and earlier, LTS 2.277.1 and earlier does not validate the type of object created after loading the data submitted to the `config.xml` REST API endpoint of a node. This allows attackers with Computer/Configure permission to replace a node with one of a different type. Jenkins 2.287, LTS 2.277.2 validates the type of object created and rejects objects of unexpected types.

The fix

[SECURITY-1721]

Matt Sicker· Mar 25, 2021, 07:23 AM+58−184210baed0
core/src/main/java/hudson/model/Computer.java+9 −1
@@ -1545,8 +1545,16 @@ public void doConfigDotXml(StaplerRequest req, StaplerResponse rsp)
      */
     public void updateByXml(final InputStream source) throws IOException, ServletException {
         checkPermission(CONFIGURE);
+        Node previous = getNode();
+        if (previous == null) {
+            throw HttpResponses.notFound();
+        }
         Node result = (Node)Jenkins.XSTREAM2.fromXML(source);
-        Jenkins.get().getNodesObject().replaceNode(this.getNode(), result);
+        if (previous.getClass() != result.getClass()) {
+            // ensure node type doesn't change
+            throw HttpResponses.errorWithoutStack(SC_BAD_REQUEST, "Node types do not match");
+        }
+        Jenkins.get().getNodesObject().replaceNode(previous, result);
     }
  
     /**
test/src/test/java/hudson/model/ComputerConfigDotXmlSEC1721Test.java+49 −0
@@ -0,0 +1,49 @@
+package hudson.model;
+ 
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.WebResponse;
+import hudson.slaves.DumbSlave;
+import jenkins.model.Jenkins;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.FakeLauncher;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.PretendSlave;
+ 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
+ 
+public class ComputerConfigDotXmlSEC1721Test {
+ 
+    @Rule
+    public final JenkinsRule j = new JenkinsRule();
+ 
+    @Rule
+    public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+ 
+    @Issue("SECURITY-1721")
+    @Test
+    public void cannotChangeNodeType() throws Exception {
+        PretendSlave agent = j.createPretendSlave(p -> new FakeLauncher.FinishedProc(0));
+        String name = agent.getNodeName();
+        assertThat(name, is(not(emptyOrNullString())));
+        Computer computer = agent.toComputer();
+        assertThat(computer, is(notNullValue()));
+ 
+        JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false);
+        WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", computer.getUrl())), HttpMethod.POST);
+        req.setAdditionalHeader("Content-Type", "application/xml");
+        // to ensure maximum compatibility of payload, we'll serialize a real one with the same name
+        DumbSlave mole = new DumbSlave(name, temporaryFolder.newFolder().getPath(), j.createComputerLauncher(null));
+        req.setRequestBody(Jenkins.XSTREAM.toXML(mole));
+        WebResponse response = wc.getPage(req).getWebResponse();
+        assertThat(response.getStatusCode(), is(400));
+ 
+        // verify node hasn't been transformed into a DumbSlave
+        Node node = j.jenkins.getNode(name);
+        assertThat(node, instanceOf(PretendSlave.class));
+    }
+}

References