diff --git a/core/src/main/java/hudson/model/Api.java b/core/src/main/java/hudson/model/Api.java index 71b617e46f..c96066d2ad 100644 --- a/core/src/main/java/hudson/model/Api.java +++ b/core/src/main/java/hudson/model/Api.java @@ -23,6 +23,7 @@ */ package hudson.model; +import jenkins.dom4j.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 list = (List)dom.selectNodes(exclude); + XPath xExclude = dom.createXPath(exclude); + xExclude.setFunctionContext(functionContext); + List list = (List)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) { diff --git a/core/src/main/java/jenkins/dom4j/FilteredFunctionContext.java b/core/src/main/java/jenkins/dom4j/FilteredFunctionContext.java new file mode 100644 index 0000000000..9307d9fa0e --- /dev/null +++ b/core/src/main/java/jenkins/dom4j/FilteredFunctionContext.java @@ -0,0 +1,49 @@ +package jenkins.dom4j; + +import org.jaxen.Function; +import org.jaxen.FunctionContext; +import org.jaxen.UnresolvableException; +import org.jaxen.XPathFunctionContext; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +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 + */ +public class FilteredFunctionContext implements FunctionContext { + + /** + * Default set of "bad" function names. + */ + private static final Set DEFAULT_ILLEGAL_FUNCTIONS = Collections.unmodifiableSet(new HashSet( + Arrays.asList("document") + )); + private final FunctionContext base; + private final Set illegalFunctions; + + public FilteredFunctionContext(Set 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())) { + throw new UnresolvableException("Illegal function: " + localName); + } + return base.getFunction(namespaceURI, prefix, localName); + } +} diff --git a/test/src/test/java/hudson/model/ApiTest.java b/test/src/test/java/hudson/model/ApiTest.java index 3b2d82f5fd..e9c2d836a6 100644 --- a/test/src/test/java/hudson/model/ApiTest.java +++ b/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,28 @@ public class ApiTest extends HudsonTestCase { new WebClient().goTo("api/xml?xpath=/*[1]","application/xml"); } + /** + * Test that calling the xml API with the xpath document function fails. + * + * See SECURITY-165 + * @throws IOException if so + * @throws SAXException if so + */ + public void testXPathDocumentFunction() throws IOException, SAXException { + 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");