Merge pull request #9 from jenkinsci-cert/SECURITY-165

[SECURITY-165] blacklist the document xpath function for use in Api

core/src/main/java/hudson/model/Api.java

@@ -23,6 +23,7 @@
  */
 package hudson.model;
 
+import jenkins.util.xml.FilteredFunctionContext;
 import jenkins.model.Jenkins;
 import jenkins.security.SecureRequester;
 
@@ -31,6 +32,7 @@ import org.dom4j.Document;
 import org.dom4j.DocumentException;
 import org.dom4j.DocumentFactory;
 import org.dom4j.Element;
+import org.dom4j.XPath;
 import org.dom4j.io.SAXReader;
 import org.dom4j.io.XMLWriter;
 import org.kohsuke.stapler.QueryParameter;
@@ -106,14 +108,16 @@ public class Api extends AbstractModelObject {
         p.writeTo(bean,pruner,Flavor.XML.createDataWriter(bean,sw));
 
         // apply XPath
+        FilteredFunctionContext functionContext = new FilteredFunctionContext();
         Object result;
         try {
             Document dom = new SAXReader().read(new StringReader(sw.toString()));
-
             // apply exclusions
             if (excludes!=null) {
                 for (String exclude : excludes) {
-                    List<org.dom4j.Node> list = (List<org.dom4j.Node>)dom.selectNodes(exclude);
+                    XPath xExclude = dom.createXPath(exclude);
+                    xExclude.setFunctionContext(functionContext);
+                    List<org.dom4j.Node> list = (List<org.dom4j.Node>)xExclude.selectNodes(dom);
                     for (org.dom4j.Node n : list) {
                         Element parent = n.getParent();
                         if(parent!=null)
@@ -125,7 +129,9 @@ public class Api extends AbstractModelObject {
             if(xpath==null) {
             	result = dom;
             } else {
-                List list = dom.selectNodes(xpath);
+                XPath comp = dom.createXPath(xpath);
+                comp.setFunctionContext(functionContext);
+                List list = comp.selectNodes(dom);
                 if (wrapper!=null) {
                     Element root = DocumentFactory.getInstance().createElement(wrapper);
                     for (Object o : list) {

core/src/main/java/jenkins/util/xml/FilteredFunctionContext.java

+
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015, CloudBees, Inc. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.util.xml;
+
+import org.jaxen.Function;
+import org.jaxen.FunctionContext;
+import org.jaxen.UnresolvableException;
+import org.jaxen.XPathFunctionContext;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+/**
+ * {@link org.jaxen.FunctionContext} that removes some {@link org.dom4j.XPath}
+ * function names that are deemed bad as user input.
+ *
+ * @author Robert Sandell <rsandell@cloudbees.com>.
+ * @see org.jaxen.FunctionContext
+ * @see org.dom4j.XPath
+ * @see hudson.model.Api
+ */
+@Restricted(NoExternalUse.class)
+public class FilteredFunctionContext implements FunctionContext {
+
+    /**
+     * Default set of "bad" function names.
+     */
+    private static final Set<String> DEFAULT_ILLEGAL_FUNCTIONS = Collections.unmodifiableSet(new HashSet<String>(
+            Arrays.asList("document")
+    ));
+    private final FunctionContext base;
+    private final Set<String> illegalFunctions;
+
+    public FilteredFunctionContext(Set<String> illegalFunctions) {
+        this.illegalFunctions = illegalFunctions;
+        base = XPathFunctionContext.getInstance();
+    }
+
+    public FilteredFunctionContext() {
+        this(DEFAULT_ILLEGAL_FUNCTIONS);
+    }
+
+    @Override
+    public Function getFunction(String namespaceURI, String prefix, String localName) throws UnresolvableException {
+        if (localName != null && illegalFunctions.contains(localName.toLowerCase(Locale.ENGLISH))) {
+            throw new UnresolvableException("Illegal function: " + localName);
+        }
+        return base.getFunction(namespaceURI, prefix, localName);
+    }
+}

test/src/test/java/hudson/model/ApiTest.java

@@ -24,9 +24,13 @@
 package hudson.model;
 
 import com.gargoylesoftware.htmlunit.Page;
+
+import java.io.File;
+import java.io.IOException;
 import java.net.HttpURLConnection;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.Bug;
+import org.xml.sax.SAXException;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -38,6 +42,27 @@ public class ApiTest extends HudsonTestCase {
         new WebClient().goTo("api/xml?xpath=/*[1]","application/xml");
     }
 
+    /**
+     * Test that calling the XML API with the XPath <code>document</code> function fails.
+     *
+     * @throws Exception if so
+     */
+    //Issue("SECURITY-165")
+    public void testXPathDocumentFunction() throws Exception {
+        File f = new File(jenkins.getRootDir(), "queue.xml");
+        WebClient client = createWebClient();
+
+        try {
+            client.goTo("api/xml?xpath=document(\"" + f.getAbsolutePath() + "\")", "application/xml");
+            fail("Should become 500 error");
+        } catch (com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException e) {
+            String contentAsString = e.getResponse().getContentAsString();
+            assertStringContains(
+                    contentAsString,
+                    "Illegal function: document");
+        }
+    }
+
     @Bug(3267)
     public void testWrappedZeroItems() throws Exception {
         Page page = new WebClient().goTo("api/xml?wrapper=root&xpath=/hudson/nonexistent", "application/xml");