CVE-2018-1000407

@@ -133,7 +133,19 @@ public void doXml(StaplerRequest req, StaplerResponse rsp,
                 XPath comp = dom.createXPath(xpath);
                 comp.setFunctionContext(functionContext);
                 List list = comp.selectNodes(dom);
+
                 if (wrapper!=null) {
+                    // check if the wrapper is a valid entity name
+                    // First position:  letter or underscore
+                    // Other positions: \w (letter, number, underscore), dash or dot
+                    String validNameRE = "^[a-zA-Z_][\\w-\\.]*$";
+
+                    if(!wrapper.matches(validNameRE)) {
+                        rsp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+                        rsp.getWriter().print(Messages.Api_WrapperParamInvalid());
+                        return;
+                    }
+
                     Element root = DocumentFactory.getInstance().createElement(wrapper);
                     for (Object o : list) {
                         if (o instanceof String) {

@@ -47,6 +47,7 @@ AbstractProject.ExtendedReadPermission.Description=\
 Api.MultipleMatch=XPath "{0}" encontr\u00f3 coincidencias en {1} nodos. \
   Crea una expresi\u00f3n XPath que s\u00f3lo encuentre uno, o utiliza el par\u00e1metro "wraper" para que todos los nodos se agrupen bajo un elemento.
 Api.NoXPathMatch=XPath {0} no encontr\u00f3 nada 
+Api.WrapperParamInvalid=El par\u00E1metro wrapper s\u00F3lo puede contener caracteres alfanum\u00E9ricos o guiones/puntos/subrayado. El primer car\u00E1cter debe ser una letra o subrayado.
 
 BallColor.Aborted=Abortado
 BallColor.Disabled=Desactivado

@@ -41,6 +41,7 @@ AbstractProject.WorkspacePermission.Description=\
 Api.MultipleMatch=Le XPath "{0}" correspond \u00e0 {1} noeud(s). \
     Merci de fournir un XPath qui ne correspond qu''\u00e0 un seul noeud, ou utilisez le param\u00e8tre de requ\u00e8te "wrapper" pour les encapsuler tous dans un \u00e9l\u00e9ment racine.
 Api.NoXPathMatch=Pas de correspondance avec le XPath {0}
+Api.WrapperParamInvalid=Le param\u00E8tre wrapper ne doit contenir que des caract\u00E8res alphanum\u00E9riques ainsi que des points / tirets bas / traits d''union, et finalement le premier caract\u00E8re doit \u00EAtre une lettre ou un trait d''union.
 
 BallColor.Aborted=Annul\u00e9
 BallColor.Disabled=D\u00e9sactiv\u00e9
@@ -178,3 +179,4 @@ MyViewsProperty.GlobalAction.DisplayName=Mes vues
 
 ManageJenkinsAction.DisplayName=Administrer Jenkins
 ParametersDefinitionProperty.DisplayName=Ce build a des param�tres
+

@@ -84,6 +84,7 @@ AbstractProject.LabelLink=<a href="{0}{2}">Label {1}</a> is serviced by {3,choic
 Api.MultipleMatch=XPath "{0}" matched {1} nodes. \
     Create XPath that only matches one, or use the "wrapper" query parameter to wrap them all under a root element.
 Api.NoXPathMatch=XPath {0} didn\u2019t match
+Api.WrapperParamInvalid=The wrapper parameter can only contain alphanumeric characters or dash/dot/underscore. The first character has to be a letter or underscore.
 
 BallColor.Aborted=Aborted
 BallColor.Disabled=Disabled

@@ -0,0 +1,86 @@
+package hudson.model;
+
+import com.gargoylesoftware.htmlunit.WebResponse;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.xml.sax.SAXException;
+
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+//TODO after the security fix, it could be merged inside ApiTest
+public class ApiSecurity1129Test {
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    /**
+     * Test the wrapper parameter for the api/xml urls to avoid XSS.
+     * @throws Exception See {@link #checkWrapperParam(String, Integer, String)}
+     */
+    @Issue("SECURITY-1129")
+    @Test
+    public void wrapperXss() throws Exception {
+        String wrapper = "html%20xmlns=\"http://www.w3.org/1999/xhtml\"><script>alert(%27XSS%20Detected%27)</script></html><!--";
+
+        checkWrapperParam(wrapper, HttpServletResponse.SC_BAD_REQUEST, Messages.Api_WrapperParamInvalid());
+    }
+
+    /**
+     * Test the wrapper parameter for the api/xml urls with a bad name.
+     * @throws Exception See {@link #checkWrapperParam(String, Integer, String)}
+     */
+    @Issue("SECURITY-1129")
+    @Test
+    public void wrapperBadName() throws Exception {
+        String wrapper = "-badname";
+        checkWrapperParam(wrapper, HttpServletResponse.SC_BAD_REQUEST, Messages.Api_WrapperParamInvalid());
+
+    }
+
+    /**
+     * Test thw erapper parameter with a good name, to ensure the security fix doesn't break anything.
+     * @throws Exception See {@link #checkWrapperParam(String, Integer, String)}
+     */
+    @Issue("SECURITY-1129")
+    @Test
+    public void wrapperGoodName() throws Exception {
+        String wrapper = "__GoodName-..-OK";
+        checkWrapperParam(wrapper, HttpServletResponse.SC_OK, null);
+
+    }
+
+    /**
+     * Check the response for a XML api with the wrapper param specified. At least the statuCode or the responseMessage
+     * should be indicated.
+     * @param wrapper the wrapper param passed in theurl.
+     * @param statusCode the status code expected in the response. If it's null, it's not checked.
+     * @param responseMessage the message expected in the response. If it's null, it's not checked.
+     * @throws IOException See {@link org.jvnet.hudson.test.JenkinsRule.WebClient#goTo(String, String)}
+     * @throws SAXException See {@link org.jvnet.hudson.test.JenkinsRule.WebClient#goTo(String, String)}
+     */
+    private void checkWrapperParam(String wrapper, Integer statusCode, String responseMessage) throws IOException, SAXException {
+        if (statusCode == null && responseMessage == null) {
+            fail("You should check at least one, the statusCode or the responseMessage when testing the wrapper param");
+        }
+
+        JenkinsRule.WebClient wc = j.createWebClient();
+        wc.getOptions().setThrowExceptionOnFailingStatusCode(false);
+        WebResponse response = wc.goTo(String.format("whoAmI/api/xml?xpath=*&wrapper=%s", wrapper), null).getWebResponse();
+
+        if (response != null) {
+            if (statusCode != null) {
+                assertEquals(statusCode.intValue(), response.getStatusCode());
+            }
+            if (responseMessage != null) {
+                assertEquals(responseMessage, response.getContentAsString());
+            }
+        } else {
+            fail("The response shouldn't be null");
+        }
+    }
+}