Bug#954862: buster-pu: package checkstyle/8.15-1
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
Hello,
I would like to fix CVE-2019-9658 and CVE-2019-10782 in checkstyle.
The security team marked this issue as no-dsa. Please find attached
the debdiff for Buster.
Regards,
Markus
diff -Nru checkstyle-8.15/debian/changelog checkstyle-8.15/debian/changelog
--- checkstyle-8.15/debian/changelog 2018-12-18 13:50:05.000000000 +0100
+++ checkstyle-8.15/debian/changelog 2020-03-24 14:03:07.000000000 +0100
@@ -1,3 +1,14 @@
+checkstyle (8.15-1+deb10u1) buster; urgency=medium
+
+ * Team upload.
+ * Fix CVE-2019-9658 and CVE-2019-10782:
+ Security researchers from Snyk discovered that the fix for CVE-2019-9658
+ was incomplete. Checkstyle, a development tool to help programmers write
+ Java code that adheres to a coding standard, was still vulnerable to XML
+ External Entity (XXE) injection. (Closes: #924598)
+
+ -- Markus Koschany <apo@debian.org> Tue, 24 Mar 2020 14:03:07 +0100
+
checkstyle (8.15-1) unstable; urgency=medium
* Team upload.
diff -Nru checkstyle-8.15/debian/patches/CVE-2019-9658-and-CVE-2019-10782.patch checkstyle-8.15/debian/patches/CVE-2019-9658-and-CVE-2019-10782.patch
--- checkstyle-8.15/debian/patches/CVE-2019-9658-and-CVE-2019-10782.patch 1970-01-01 01:00:00.000000000 +0100
+++ checkstyle-8.15/debian/patches/CVE-2019-9658-and-CVE-2019-10782.patch 2020-03-24 14:03:07.000000000 +0100
@@ -0,0 +1,184 @@
+From: Markus Koschany <apo@debian.org>
+Date: Tue, 24 Mar 2020 13:01:27 +0100
+Subject: CVE-2019-9658 and CVE-2019-10782
+
+Bug-Debian: https://bugs.debian.org/924598
+Origin: https://github.com/checkstyle/checkstyle/commit/180b4fe37a2249d4489d584505f2b7b3ab162ec6
+Origin: https://github.com/romani/checkstyle/commit/3af187f81ab33c9a8e471cc629ff10fe722a7a56
+---
+ config/pmd.xml | 4 ++--
+ pom.xml | 4 ++--
+ .../com/puppycrawl/tools/checkstyle/XmlLoader.java | 23 +++++++++++++++++-----
+ .../tools/checkstyle/ConfigurationLoaderTest.java | 9 +++++++++
+ .../puppycrawl/tools/checkstyle/XmlLoaderTest.java | 2 +-
+ src/xdocs/config_reporting.xml | 11 +++++++++++
+ 6 files changed, 43 insertions(+), 10 deletions(-)
+
+diff --git a/config/pmd.xml b/config/pmd.xml
+index 29c5c41..a715cae 100644
+--- a/config/pmd.xml
++++ b/config/pmd.xml
+@@ -96,13 +96,13 @@
+ </rule>
+ <rule ref="category/java/codestyle.xml/ClassNamingConventions">
+ <properties>
+- <!-- Definitions and XmlLoader.FeaturesForVerySecureJavaInstallations aren't utility classes.
++ <!-- Definitions and XmlLoader.LoadExternalDtdFeatureProvider aren't utility classes.
+ JavadocTokenTypes and TokenTypes aren't utility classes.
+ They are token definition classes. Also, they are part of the API. -->
+ <property name="violationSuppressXPath"
+ value="//ClassOrInterfaceDeclaration[@Image='Definitions'
+ or @Image='JavadocTokenTypes' or @Image='TokenTypes'
+- or @Image='FeaturesForVerySecureJavaInstallations']"/>
++ or @Image='LoadExternalDtdFeatureProvider']"/>
+ </properties>
+ </rule>
+ <rule ref="category/java/codestyle.xml/ConfusingTernary">
+diff --git a/pom.xml b/pom.xml
+index 8005c34..bcdcd86 100644
+--- a/pom.xml
++++ b/pom.xml
+@@ -2526,12 +2526,12 @@
+ </targetTests>
+ <excludedMethods>
+ <!--cause of https://github.com/checkstyle/checkstyle/issues/3605-->
+- <param>addFeaturesForVerySecureJavaInstallations</param>
++ <param>setFeaturesBySystemProperty</param>
+ </excludedMethods>
+ <avoidCallsTo>
+ <!--cause of https://github.com/checkstyle/checkstyle/issues/3605-->
+ <avoidCallsTo>
+- com.puppycrawl.tools.checkstyle.XmlLoader$FeaturesForVerySecureJavaInstallations
++ com.puppycrawl.tools.checkstyle.XmlLoader$LoadExternalDtdFeatureProvider
+ </avoidCallsTo>
+ </avoidCallsTo>
+ <coverageThreshold>99</coverageThreshold>
+diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/XmlLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/XmlLoader.java
+index 71d61be..56bdc67 100644
+--- a/src/main/java/com/puppycrawl/tools/checkstyle/XmlLoader.java
++++ b/src/main/java/com/puppycrawl/tools/checkstyle/XmlLoader.java
+@@ -65,7 +65,7 @@ public class XmlLoader
+ throws SAXException, ParserConfigurationException {
+ this.publicIdToResourceNameMap = new HashMap<>(publicIdToResourceNameMap);
+ final SAXParserFactory factory = SAXParserFactory.newInstance();
+- FeaturesForVerySecureJavaInstallations.addFeaturesForVerySecureJavaInstallations(factory);
++ LoadExternalDtdFeatureProvider.setFeaturesBySystemProperty(factory);
+ factory.setValidating(true);
+ factory.setNamespaceAware(true);
+ parser = factory.newSAXParser().getXMLReader();
+@@ -119,7 +119,10 @@ public class XmlLoader
+ * Used for setting specific for secure java installations features to SAXParserFactory.
+ * Pulled out as a separate class in order to suppress Pitest mutations.
+ */
+- public static final class FeaturesForVerySecureJavaInstallations {
++ public static final class LoadExternalDtdFeatureProvider {
++
++ /** System property name to enable external DTD load. */
++ public static final String ENABLE_EXTERNAL_DTD_LOAD = "checkstyle.enableExternalDtdLoad";
+
+ /** Feature that enables loading external DTD when loading XML files. */
+ private static final String LOAD_EXTERNAL_DTD =
+@@ -127,22 +130,32 @@ public class XmlLoader
+ /** Feature that enables including external general entities in XML files. */
+ private static final String EXTERNAL_GENERAL_ENTITIES =
+ "http://xml.org/sax/features/external-general-entities";;
++ /** Feature that enables including external parameter entities in XML files. */
++ public static final String EXTERNAL_PARAMETER_ENTITIES =
++ "http://xml.org/sax/features/external-parameter-entities";;
+
+ /** Stop instances being created. **/
+- private FeaturesForVerySecureJavaInstallations() {
++ private LoadExternalDtdFeatureProvider() {
+ }
+
+ /**
+ * Configures SAXParserFactory with features required
+- * for execution on very secured environments.
++ * to use external DTD file loading, this is not activated by default to no allow
++ * usage of schema files that checkstyle do not know
++ * it is even security problem to allow files from outside.
+ * @param factory factory to be configured with special features
+ * @throws SAXException if an error occurs
+ * @throws ParserConfigurationException if an error occurs
+ */
+- public static void addFeaturesForVerySecureJavaInstallations(SAXParserFactory factory)
++ public static void setFeaturesBySystemProperty(SAXParserFactory factory)
+ throws SAXException, ParserConfigurationException {
++
++ final boolean enableExternalDtdLoad = Boolean.valueOf(
++ System.getProperty(ENABLE_EXTERNAL_DTD_LOAD, "false"));
++
+ factory.setFeature(LOAD_EXTERNAL_DTD, true);
+ factory.setFeature(EXTERNAL_GENERAL_ENTITIES, true);
++ factory.setFeature(EXTERNAL_PARAMETER_ENTITIES, enableExternalDtdLoad);
+ }
+
+ }
+diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java
+index eb54a96..73f2542 100644
+--- a/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java
++++ b/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java
+@@ -413,6 +413,9 @@ public class ConfigurationLoaderTest extends AbstractPathTestSupport {
+ final Properties props = new Properties();
+ props.setProperty("checkstyle.basedir", "basedir");
+
++ System.setProperty(
++ XmlLoader.LoadExternalDtdFeatureProvider.ENABLE_EXTERNAL_DTD_LOAD, "true");
++
+ final DefaultConfiguration config =
+ (DefaultConfiguration) loadConfiguration(
+ "InputConfigurationLoaderExternalEntity.xml", props);
+@@ -428,6 +431,9 @@ public class ConfigurationLoaderTest extends AbstractPathTestSupport {
+ final Properties props = new Properties();
+ props.setProperty("checkstyle.basedir", "basedir");
+
++ System.setProperty(
++ XmlLoader.LoadExternalDtdFeatureProvider.ENABLE_EXTERNAL_DTD_LOAD, "true");
++
+ final DefaultConfiguration config =
+ (DefaultConfiguration) loadConfiguration(
+ "subdir/InputConfigurationLoaderExternalEntitySubDir.xml", props);
+@@ -443,6 +449,9 @@ public class ConfigurationLoaderTest extends AbstractPathTestSupport {
+ final Properties props = new Properties();
+ props.setProperty("checkstyle.basedir", "basedir");
+
++ System.setProperty(
++ XmlLoader.LoadExternalDtdFeatureProvider.ENABLE_EXTERNAL_DTD_LOAD, "true");
++
+ final File file = new File(
+ getPath("subdir/InputConfigurationLoaderExternalEntitySubDir.xml"));
+ final DefaultConfiguration config =
+diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/XmlLoaderTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/XmlLoaderTest.java
+index cb3c914..16b6d80 100644
+--- a/src/test/java/com/puppycrawl/tools/checkstyle/XmlLoaderTest.java
++++ b/src/test/java/com/puppycrawl/tools/checkstyle/XmlLoaderTest.java
+@@ -45,7 +45,7 @@ public class XmlLoaderTest {
+ @Test
+ public void testIsProperUtilsClass() throws ReflectiveOperationException {
+ assertTrue("Constructor is not private", isUtilsClassHasPrivateConstructor(
+- XmlLoader.FeaturesForVerySecureJavaInstallations.class, true));
++ XmlLoader.LoadExternalDtdFeatureProvider.class, true));
+ }
+
+ private static final class DummyLoader extends XmlLoader {
+diff --git a/src/xdocs/config_reporting.xml b/src/xdocs/config_reporting.xml
+index a2c2584..c5f868a 100644
+--- a/src/xdocs/config_reporting.xml
++++ b/src/xdocs/config_reporting.xml
+@@ -69,5 +69,16 @@
+ to an empty string.
+ </p>
+ </section>
++
++ <section name="Enable External DTD load">
++ <p>
++ The property <code>checkstyle.enableExternalDtdLoad</code>
++ defines ability use custom DTD files inconfig and load them from some location.
++ The property type
++ is <a href="property_types.html#boolean">boolean</a> and defaults
++ to <code>false</code>.
++ </p>
++ </section>
++
+ </body>
+ </document>
diff -Nru checkstyle-8.15/debian/patches/series checkstyle-8.15/debian/patches/series
--- checkstyle-8.15/debian/patches/series 2018-12-18 11:48:17.000000000 +0100
+++ checkstyle-8.15/debian/patches/series 2020-03-24 14:03:07.000000000 +0100
@@ -1,3 +1,4 @@
01_link_to_system_javadocs.diff
02_ignore_tests_requiring_internet_connectivity.diff
05_antlr_compatibility.diff
+CVE-2019-9658-and-CVE-2019-10782.patch
Reply to: