From e7cb858852c05d2423e3fd9922a090982dcd6392 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Mon, 27 May 2019 22:11:07 +0200 Subject: [PATCH] [SECURITY-1409] --- .../pipeline/maven/MavenSpyLogProcessor.java | 39 ++++++++++++++++--- .../plugins/pipeline/maven/util/XmlUtils.java | 27 +++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/MavenSpyLogProcessor.java b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/MavenSpyLogProcessor.java index e7cf74e5..f4281f61 100644 --- a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/MavenSpyLogProcessor.java +++ b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/MavenSpyLogProcessor.java @@ -26,32 +26,30 @@ import hudson.FilePath; import hudson.model.Run; -import hudson.model.StreamBuildListener; import hudson.model.TaskListener; import jenkins.model.InterruptedBuildAction; import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.pipeline.maven.publishers.JenkinsMavenEventSpyLogsPublisher; +import org.jenkinsci.plugins.pipeline.maven.util.XmlUtils; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.w3c.dom.Element; import org.xml.sax.SAXException; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; import java.io.PrintWriter; import java.io.Serializable; import java.util.AbstractMap; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -78,7 +76,38 @@ public void processMavenSpyLogs(@Nonnull StepContext context, @Nonnull FilePath DocumentBuilder documentBuilder; try { - documentBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + + // https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.md#jaxp-documentbuilderfactory-saxparserfactory-and-dom4j + dbf.setExpandEntityReferences(false); + dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + + // This is the PRIMARY defense. If DTDs (doctypes) are disallowed, almost all + // XML entity attacks are prevented + // Xerces 2 only - http://xerces.apache.org/xerces2-j/features.html#disallow-doctype-decl + dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + + // If you can't completely disable DTDs, then at least do the following: + // Xerces 1 - http://xerces.apache.org/xerces-j/features.html#external-general-entities + // Xerces 2 - http://xerces.apache.org/xerces2-j/features.html#external-general-entities + // JDK7+ - http://xml.org/sax/features/external-general-entities + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + + // Xerces 1 - http://xerces.apache.org/xerces-j/features.html#external-parameter-entities + // Xerces 2 - http://xerces.apache.org/xerces2-j/features.html#external-parameter-entities + // JDK7+ - http://xml.org/sax/features/external-parameter-entities + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + + // Disable external DTDs as well + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + + // and these as well, per Timothy Morgan's 2014 paper: "XML Schema, DTD, and Entity Attacks" + dbf.setXIncludeAware(false); + + documentBuilder = dbf.newDocumentBuilder(); + + // See https://github.com/jenkinsci/jenkins/blob/jenkins-2.176/core/src/main/java/jenkins/util/xml/XMLUtils.java#L114 + documentBuilder.setEntityResolver(XmlUtils.RestrictiveEntityResolver.INSTANCE); } catch (ParserConfigurationException e) { throw new IllegalStateException("Failure to create a DocumentBuilder", e); } diff --git a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/util/XmlUtils.java b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/util/XmlUtils.java index 7aecc30f..572ad972 100644 --- a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/util/XmlUtils.java +++ b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/util/XmlUtils.java @@ -33,8 +33,12 @@ import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import org.xml.sax.EntityResolver; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; import java.io.File; +import java.io.IOException; import java.io.StringWriter; import java.nio.file.Path; import java.util.ArrayList; @@ -510,4 +514,27 @@ public static List listGeneratedArtifacts(Element mavenSpyLogs, b return result; } + + /** + * Copy {@link jenkins.util.xml.RestrictiveEntityResolver} as it is secured by {@link org.kohsuke.accmod.restrictions.NoExternalUse}. + * + * @see jenkins.util.xml.RestrictiveEntityResolver + */ + public final static class RestrictiveEntityResolver implements EntityResolver { + + public final static RestrictiveEntityResolver INSTANCE = new RestrictiveEntityResolver(); + + private RestrictiveEntityResolver() { + // prevent multiple instantiation. + super(); + } + + /** + * Throws a SAXException if this tried to resolve any entity. + */ + @Override + public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException { + throw new SAXException("Refusing to resolve entity with publicId(" + publicId + ") and systemId (" + systemId + ")"); + } + } }