From f1a5fdc26ac09239b3e55a9f28dc4396d0b038f4 Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Mon, 1 Mar 2021 16:02:30 +1000 Subject: [PATCH 1/2] Upgrade commons-digester 3.2 Signed-off-by: olivier lamy --- bom/pom.xml | 6 +++--- core/pom.xml | 12 ++++++++---- core/src/main/java/hudson/util/Digester2.java | 8 ++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/bom/pom.xml b/bom/pom.xml index 8572cc210f55..560ba3e90be8 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -229,9 +229,9 @@ THE SOFTWARE. 1.0.19 - commons-digester - commons-digester - 2.1 + org.apache.commons + commons-digester3 + 3.2 commons-beanutils diff --git a/core/pom.xml b/core/pom.xml index 6c1d2e1677cd..2ab82aa70627 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -274,12 +274,16 @@ THE SOFTWARE. commons-lang - commons-digester - commons-digester + org.apache.commons + commons-digester3 - xml-apis - xml-apis + commons-logging + commons-logging + + + cglib + cglib diff --git a/core/src/main/java/hudson/util/Digester2.java b/core/src/main/java/hudson/util/Digester2.java index 23aa7f50acda..5a1d890a5b0a 100644 --- a/core/src/main/java/hudson/util/Digester2.java +++ b/core/src/main/java/hudson/util/Digester2.java @@ -24,8 +24,8 @@ package hudson.util; import jenkins.util.SystemProperties; -import org.apache.commons.digester.Digester; -import org.apache.commons.digester.Rule; +import org.apache.commons.digester3.Digester; +import org.apache.commons.digester3.Rule; import org.xml.sax.Attributes; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -152,12 +152,12 @@ private static final class ObjectCreateRule2 extends Rule { @Override public void begin(String namespace, String name, Attributes attributes) throws Exception { Object instance = clazz.newInstance(); - digester.push(instance); + getDigester().push(instance); } @Override public void end(String namespace, String name) throws Exception { - digester.pop(); + getDigester().pop(); } } } From a376fa2ca4187994cd14d70285cf589c15997115 Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Tue, 30 Mar 2021 12:01:56 +1000 Subject: [PATCH 2/2] remove Digester from core Signed-off-by: olivier lamy --- bom/pom.xml | 5 - core/pom.xml | 14 -- core/src/main/java/hudson/util/Digester2.java | 163 ------------------ .../test/java/hudson/util/Digester2Test.java | 85 --------- 4 files changed, 267 deletions(-) delete mode 100644 core/src/main/java/hudson/util/Digester2.java delete mode 100644 core/src/test/java/hudson/util/Digester2Test.java diff --git a/bom/pom.xml b/bom/pom.xml index 560ba3e90be8..f28f007d634d 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -228,11 +228,6 @@ THE SOFTWARE. jfreechart 1.0.19 - - org.apache.commons - commons-digester3 - 3.2 - commons-beanutils commons-beanutils diff --git a/core/pom.xml b/core/pom.xml index 2ab82aa70627..a052a26350d5 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -273,20 +273,6 @@ THE SOFTWARE. commons-lang commons-lang - - org.apache.commons - commons-digester3 - - - commons-logging - commons-logging - - - cglib - cglib - - - commons-beanutils commons-beanutils diff --git a/core/src/main/java/hudson/util/Digester2.java b/core/src/main/java/hudson/util/Digester2.java deleted file mode 100644 index 5a1d890a5b0a..000000000000 --- a/core/src/main/java/hudson/util/Digester2.java +++ /dev/null @@ -1,163 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi - * - * 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 hudson.util; - -import jenkins.util.SystemProperties; -import org.apache.commons.digester3.Digester; -import org.apache.commons.digester3.Rule; -import org.xml.sax.Attributes; -import org.xml.sax.InputSource; -import org.xml.sax.SAXException; -import org.xml.sax.XMLReader; - -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParser; -import java.io.StringReader; -import java.util.HashMap; -import java.util.Map; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * {@link Digester} wrapper to fix the issue DIGESTER-118. - * Since Jenkins 2.TODO, this class also attempts to set secure parsing defaults to prevent XXE (XML External Entity) vulnerabilities. - *
    - *
  • {@link #Digester2()} and {@link #Digester2(XMLReader)} will apply XXE protections unless a system property is set to disable them.
    • - *
  • {@link #Digester2(boolean)} and {@link #Digester2(XMLReader, boolean)} will apply XXE protections if and only if the boolean argument is true.
    • - *
  • {@link #Digester2(SAXParser)} will not apply protections, whatever instantiated the {@code SAXParser} should do that.
    • - *
- * - * @author Kohsuke Kawaguchi - * @since 1.125 - */ -// TODO deprecate and possibly restrict in a subsequent weekly release -public class Digester2 extends Digester { - - private static final Logger LOGGER = Logger.getLogger(Digester2.class.getName()); - - public Digester2() { - if (shouldConfigureSecurely()) { - configureSecurely(this); - } - } - - /** - * Callers need to configure the {@link SAXParser} securely if processing potentially untrusted input, as this does not do it automatically (unlike other constructors). - * @param parser the parser - */ - @Deprecated - public Digester2(SAXParser parser) { - super(parser); - } - - public Digester2(XMLReader reader) { - super(reader); - if (shouldConfigureSecurely()) { - configureSecurely(reader); - } - } - - /** - * @param processSecurely true iff this should configure the parser to prevent XXE. - * @since 2.275 and 2.263.2 - */ - public Digester2(boolean processSecurely) { - if (processSecurely) { - configureSecurely(this); - } - } - - /** - * @param reader the reader - * @param processSecurely true iff this should configure the parser to prevent XXE. - * @since 2.275 and 2.263.2 - */ - public Digester2(XMLReader reader, boolean processSecurely) { - super(reader); - if (processSecurely) { - configureSecurely(reader); - } - } - - private boolean shouldConfigureSecurely() { - return !SystemProperties.getBoolean(Digester2.class.getName() + ".UNSAFE"); - } - - private static void configureSecurely(Digester digester) { - digester.setXIncludeAware(false); - digester.setEntityResolver((publicId, systemId) -> new InputSource(new StringReader(""))); - digester.setNamespaceAware(false); - for (Map.Entry entry : FEATURES.entrySet()) { - try { - digester.setFeature(entry.getKey(), entry.getValue()); - } catch (ParserConfigurationException|SAXException ex) { - LOGGER.log(Level.WARNING, "Failed to securely configure Digester2 instance", ex); - } - } - } - - private static void configureSecurely(XMLReader reader) { - reader.setEntityResolver((publicId, systemId) -> new InputSource(new StringReader(""))); - for (Map.Entry entry : FEATURES.entrySet()) { - try { - reader.setFeature(entry.getKey(), entry.getValue()); - } catch (SAXException ex) { - LOGGER.log(Level.WARNING, "Failed to securely configure Digester2/XMLReader instance", ex); - } - } - } - - // TODO JDK 9+: Use Map.of instead - private static final Map FEATURES = new HashMap<>(); - static { - FEATURES.put("http://apache.org/xml/features/disallow-doctype-decl", true); - FEATURES.put("http://xml.org/sax/features/external-general-entities", false); - FEATURES.put("http://xml.org/sax/features/external-parameter-entities", false); - FEATURES.put("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - } - - @Override - public void addObjectCreate(String pattern, Class clazz) { - addRule(pattern,new ObjectCreateRule2(clazz)); - } - - private static final class ObjectCreateRule2 extends Rule { - private final Class clazz; - - ObjectCreateRule2(Class clazz) { - this.clazz = clazz; - } - - @Override - public void begin(String namespace, String name, Attributes attributes) throws Exception { - Object instance = clazz.newInstance(); - getDigester().push(instance); - } - - @Override - public void end(String namespace, String name) throws Exception { - getDigester().pop(); - } - } -} diff --git a/core/src/test/java/hudson/util/Digester2Test.java b/core/src/test/java/hudson/util/Digester2Test.java deleted file mode 100644 index 84def757eae8..000000000000 --- a/core/src/test/java/hudson/util/Digester2Test.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2020, CloudBees, Inc. - * - * 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 hudson.util; - -import org.junit.Assert; -import org.junit.Test; -import org.jvnet.hudson.test.Issue; -import org.xml.sax.SAXParseException; - -import java.io.FileNotFoundException; -import java.net.ConnectException; -import java.util.Locale; - -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.MatcherAssert.assertThat; - -public class Digester2Test { - - private static final String RESOURCE_FILE_NAME = "Digester2Security2147TestData.xml"; - - @Issue("SECURITY-2147") - @Test - public void testProtection() throws Exception { - Locale defaultLocale = Locale.getDefault(); - try { - // otherwise it's not passing on machine not running english as OS language - Locale.setDefault(Locale.ENGLISH); - try { - new Digester2().parse(Digester2Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); - Assert.fail("expected exception"); - } catch (SAXParseException ex) { - assertThat(ex.getMessage(), containsString("DOCTYPE is disallowed")); - } - } - finally { - Locale.setDefault(defaultLocale); - } - } - @Issue("SECURITY-2147") - @Test - public void testUnsafeBehavior() throws Exception { - try { - new Digester2(false).parse(Digester2Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); - Assert.fail("expected exception"); - } catch (FileNotFoundException|ConnectException ex) { - // network or file access is bad - } - } - - @Issue("SECURITY-2147") - @Test - public void testEscapeHatch() throws Exception { - final String key = Digester2.class.getName() + ".UNSAFE"; - try { - System.setProperty(key, "true"); - new Digester2().parse(Digester2Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); - Assert.fail("expected exception"); - } catch (FileNotFoundException|ConnectException ex) { - // network or file access is bad - } finally { - System.clearProperty(key); - } - } -}