Bug#921772: CVE-2018-1000652

[tony mancill]

On Fri, Feb 08, 2019 at 11:37:20PM +0100, Moritz Muehlenhoff wrote:
> Package: jabref
> Severity: grave
> Tags: security
> 
> This was assigned CVE-2018-1000652:
> https://github.com/JabRef/jabref/issues/4229
> https://github.com/JabRef/jabref/commit/89f855d76713b4cd25ac0830c719cd61c511851e

Hello Moritz,

Attached is a debdiff to address this CVE in stretch.  Please let me
know how/whether you'd like to proceed.  (I could prepare an upload for
stretch-pu instead if that's preferable.)


I have built the binary and tested locally and everything appears to be
working as expected.

Thanks to Gregor putting this together.

Cheers,
tony
-------------- next part --------------
diff -Nru jabref-3.8.1+ds/debian/changelog jabref-3.8.1+ds/debian/changelog
--- jabref-3.8.1+ds/debian/changelog	2017-01-11 12:27:19.000000000 -0800
+++ jabref-3.8.1+ds/debian/changelog	2019-02-10 11:25:26.000000000 -0800
@@ -1,3 +1,12 @@
+jabref (3.8.1+ds-3+deb9u1) stretch-security; urgency=medium
+
+  [ gregor herrmann & tony mancill ]
+  * Add patch from upstream commit to fix CVE-2018-1000652: XML External
+    Entity attack.
+    Thanks to Moritz Muehlenhoff for the bug report. (Closes: #921772)
+
+ -- gregor herrmann <gregoa at debian.org>  Sun, 10 Feb 2019 20:25:26 +0100
+
 jabref (3.8.1+ds-3) unstable; urgency=medium
 
   * Remove postgresql entry from debian/maven.rules.
diff -Nru jabref-3.8.1+ds/debian/patches/100_CVE-2018-1000652_XXE-vulnerability.patch jabref-3.8.1+ds/debian/patches/100_CVE-2018-1000652_XXE-vulnerability.patch
--- jabref-3.8.1+ds/debian/patches/100_CVE-2018-1000652_XXE-vulnerability.patch	1969-12-31 16:00:00.000000000 -0800
+++ jabref-3.8.1+ds/debian/patches/100_CVE-2018-1000652_XXE-vulnerability.patch	2019-02-10 11:25:26.000000000 -0800
@@ -0,0 +1,81 @@
+From 89f855d76713b4cd25ac0830c719cd61c511851e Mon Sep 17 00:00:00 2001
+From: Nick <nick.s.weatherley at protonmail.com>
+Date: Mon, 30 Jul 2018 16:06:07 +0000
+Subject: [PATCH] Fix importer vulnerability (#4240)
+
+* Fix importer vulnerability
+Fixed issue #4229  where importer was vulnerable to XXE attacks by
+disabling DTDs along with adding warning to logger if features are
+unavailable. fixes #4229
+
+Bugs-Debian: https://bugs.debian.org/921772
+Bug: https://github.com/JabRef/jabref/issues/4229
+
+--- a/src/main/java/net/sf/jabref/logic/importer/fileformat/MsBibImporter.java
++++ b/src/main/java/net/sf/jabref/logic/importer/fileformat/MsBibImporter.java
+@@ -6,12 +6,15 @@
+ 
+ import javax.xml.parsers.DocumentBuilder;
+ import javax.xml.parsers.DocumentBuilderFactory;
++import javax.xml.parsers.ParserConfigurationException;
+ 
+ import net.sf.jabref.logic.importer.Importer;
+ import net.sf.jabref.logic.importer.ParserResult;
+ import net.sf.jabref.logic.msbib.MSBibDatabase;
+ import net.sf.jabref.logic.util.FileExtensions;
+ 
++import org.apache.commons.logging.Log;
++import org.apache.commons.logging.LogFactory;
+ import org.w3c.dom.Document;
+ import org.xml.sax.InputSource;
+ 
+@@ -23,6 +26,10 @@
+  */
+ public class MsBibImporter extends Importer {
+ 
++    private static final Log LOGGER = LogFactory.getLog(MsBibImporter.class);
++    private static final String DISABLEDTD = "http://apache.org/xml/features/disallow-doctype-decl";
++    private static final String DISABLEEXTERNALDTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
++
+     @Override
+     public boolean isRecognizedFormat(BufferedReader reader) throws IOException {
+         Objects.requireNonNull(reader);
+@@ -34,7 +41,7 @@
+          */
+         Document docin;
+         try {
+-            DocumentBuilder dbuild = DocumentBuilderFactory.newInstance().newDocumentBuilder();
++            DocumentBuilder dbuild = makeSafeDocBuilderFactory(DocumentBuilderFactory.newInstance()).newDocumentBuilder();
+             docin = dbuild.parse(new InputSource(reader));
+         } catch (Exception e) {
+             return false;
+@@ -65,4 +72,29 @@
+         return "Importer for the MS Office 2007 XML bibliography format.";
+     }
+ 
++    /**
++     * DocumentBuilderFactory makes a XXE safe Builder factory from dBuild. If not supported by current
++     * XML then returns original builder given and logs error.
++     * @param dBuild | DocumentBuilderFactory to be made XXE safe.
++     * @return If supported, XXE safe DocumentBuilderFactory. Else, returns original builder given
++     */
++    private DocumentBuilderFactory makeSafeDocBuilderFactory(DocumentBuilderFactory dBuild) {
++        String feature = null;
++
++        try {
++            feature = DISABLEDTD;
++            dBuild.setFeature(feature, true);
++
++            feature = DISABLEEXTERNALDTD;
++            dBuild.setFeature(feature, false);
++
++            dBuild.setXIncludeAware(false);
++            dBuild.setExpandEntityReferences(false);
++
++        } catch (ParserConfigurationException e) {
++            LOGGER.warn("Builder not fully configured. Feature:'" + feature + "' is probably not supported by current XML processor.", e);
++        }
++
++        return dBuild;
++    }
+ }
diff -Nru jabref-3.8.1+ds/debian/patches/series jabref-3.8.1+ds/debian/patches/series
--- jabref-3.8.1+ds/debian/patches/series	2017-01-11 12:27:19.000000000 -0800
+++ jabref-3.8.1+ds/debian/patches/series	2019-02-10 11:25:26.000000000 -0800
@@ -4,3 +4,4 @@
 030_xjc.patch
 050_unirest_json.patch
 070_restore_normal_colors.patch
+100_CVE-2018-1000652_XXE-vulnerability.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-java-maintainers/attachments/20190411/c71dd21c/attachment.sig>