Fix importer vulnerability (JabRef#4240)

* Fix importer vulnerability
Fixed issue JabRef#4229  where importer was vulnerable to XXE attacks by
disabling DTDs along with adding warning to logger if features are
unavailable. fixes JabRef#4229

* Fix minor code errors and logger optimization
Removed author line in class comment. Reworded CHANGLOG.md. Set
DTD features to individual final static constants. Optimized
logger by parameterizing feature and error.

* Rearrange import statments for project compatibility

* Remove merge artefacts from changelog

CHANGELOG.md

@@ -60,7 +60,8 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where the "Convert to BibTeX-Cleanup" moved the content of the `file` field to the `pdf` field [#4120](https://github.com/JabRef/jabref/issues/4120)
 - We fixed an issue where the preview pane in entry preview in preferences wasn't showing the citation style selected [#3849](https://github.com/JabRef/jabref/issues/3849)
 - We fixed an issue where the default entry preview style still contained the field `review`. The field `review` in the style is now replaced with comment to be consistent with the entry editor [#4098](https://github.com/JabRef/jabref/issues/4098)
-- We fixed an issue where filles added via the "Attach file" contextmenu of an entry were not made relative. [#4201](https://github.com/JabRef/jabref/issues/4201)
+- We fixed an issue where users were vulnerable to XXE attacks during parsing [#4229](https://github.com/JabRef/jabref/issues/4229)
+- We fixed an issue where files added via the "Attach file" contextmenu of an entry were not made relative. [#4201](https://github.com/JabRef/jabref/issues/4201)
 
 
 

src/main/java/org/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 org.jabref.logic.importer.Importer;
 import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.msbib.MSBibDatabase;
 import org.jabref.logic.util.StandardFileType;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
 import org.xml.sax.ErrorHandler;
 import org.xml.sax.InputSource;
@@ -20,24 +23,27 @@
 
 /**
  * Importer for the MS Office 2007 XML bibliography format
- * By S. M. Mahbub Murshed
  *
  * ...
  */
 public class MsBibImporter extends Importer {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(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);
 
         /*
-            The correct behaviour is to return false if it is certain that the file is
+            The correct behavior is to return false if it is certain that the file is
             not of the MsBib type, and true otherwise. Returning true is the safe choice
             if not certain.
          */
         Document docin;
         try {
-            DocumentBuilder dbuild = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+            DocumentBuilder dbuild = makeSafeDocBuilderFactory(DocumentBuilderFactory.newInstance()).newDocumentBuilder();
             dbuild.setErrorHandler(new ErrorHandler() {
 
                 @Override
@@ -55,6 +61,7 @@ public void error(SAXParseException exception) throws SAXException {
                     throw exception;
                 }
             });
+
             docin = dbuild.parse(new InputSource(reader));
         } catch (Exception e) {
             return false;
@@ -85,4 +92,29 @@ public String getDescription() {
         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:'{}' is probably not supported by current XML processor. {}", feature, e);
+        }
+
+        return dBuild;
+    }
 }