From 07219eea710c9947f2623c6984ec3749c5d41f4e Mon Sep 17 00:00:00 2001 From: Vasili Novikov Date: Tue, 14 Jan 2020 18:39:14 +0100 Subject: [PATCH] backport XXE vulnerability fix Backports 8963506897e3270a75b062f28486934bcb79b1e3 Addresses CVE 2018-1000825. --- .../freecol/common/io/FreeColXMLReader.java | 20 +++++++++++++++++-- .../freecol/common/io/FreeColXMLWriter.java | 3 +++ .../freecol/common/model/FreeColObject.java | 3 +++ .../freecol/common/networking/Connection.java | 3 +++ .../freecol/common/networking/DOMMessage.java | 3 +++ .../freecol/tools/GenerateDocumentation.java | 3 +++ 6 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/net/sf/freecol/common/io/FreeColXMLReader.java b/src/net/sf/freecol/common/io/FreeColXMLReader.java index dd78a40e24..0eb56fdfe9 100644 --- a/src/net/sf/freecol/common/io/FreeColXMLReader.java +++ b/src/net/sf/freecol/common/io/FreeColXMLReader.java @@ -88,7 +88,7 @@ public FreeColXMLReader(InputStream inputStream) throws IOException { super(); try { - XMLInputFactory xif = XMLInputFactory.newInstance(); + XMLInputFactory xif = newXMLInputFactory(); setParent(xif.createXMLStreamReader(inputStream, "UTF-8")); } catch (XMLStreamException e) { throw new IOException(e); @@ -109,7 +109,7 @@ public FreeColXMLReader(Reader reader) throws IOException { super(); try { - XMLInputFactory xif = XMLInputFactory.newInstance(); + XMLInputFactory xif = newXMLInputFactory(); setParent(xif.createXMLStreamReader(reader)); } catch (XMLStreamException e) { throw new IOException(e); @@ -118,6 +118,22 @@ public FreeColXMLReader(Reader reader) throws IOException { this.readScope = ReadScope.NORMAL; } + /** + * Create a new XMLInputFactory. + * + * Respond to CVE 2018-1000825. + * + * @return A new XMLInputFactory. + */ + private static XMLInputFactory newXMLInputFactory() { + XMLInputFactory xif = XMLInputFactory.newInstance(); + // This disables DTDs entirely for that factory + xif.setProperty(XMLInputFactory.SUPPORT_DTD, false); + // disable external entities + xif.setProperty("javax.xml.stream.isSupportingExternalEntities", false); + return xif; + } + /** * Should reads from this stream intern their objects into the diff --git a/src/net/sf/freecol/common/io/FreeColXMLWriter.java b/src/net/sf/freecol/common/io/FreeColXMLWriter.java index 9c46af8f56..a517d0ba68 100644 --- a/src/net/sf/freecol/common/io/FreeColXMLWriter.java +++ b/src/net/sf/freecol/common/io/FreeColXMLWriter.java @@ -42,6 +42,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; +import javax.xml.XMLConstants; import net.sf.freecol.common.model.FreeColObject; import net.sf.freecol.common.model.Location; @@ -237,6 +238,8 @@ public void close() { .toString())); result = new StreamResult(this.outputWriter); factory = TransformerFactory.newInstance(); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); transformer = factory.newTransformer(); for (int i = 0; i < indentProps.length; i += 2) { transformer.setOutputProperty(indentProps[i], diff --git a/src/net/sf/freecol/common/model/FreeColObject.java b/src/net/sf/freecol/common/model/FreeColObject.java index 01c9887181..d8f3754a25 100644 --- a/src/net/sf/freecol/common/model/FreeColObject.java +++ b/src/net/sf/freecol/common/model/FreeColObject.java @@ -49,6 +49,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; +import javax.xml.XMLConstants; import net.sf.freecol.common.ObjectWithId; import net.sf.freecol.common.io.FreeColXMLReader; @@ -895,6 +896,8 @@ public static String readId(Element element) { public void readFromXMLElement(Element element) { try { TransformerFactory factory = TransformerFactory.newInstance(); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); Transformer xmlTransformer = factory.newTransformer(); StringWriter stringWriter = new StringWriter(); xmlTransformer.transform(new DOMSource(element), diff --git a/src/net/sf/freecol/common/networking/Connection.java b/src/net/sf/freecol/common/networking/Connection.java index f88d2eda3c..48954bd7c7 100644 --- a/src/net/sf/freecol/common/networking/Connection.java +++ b/src/net/sf/freecol/common/networking/Connection.java @@ -40,6 +40,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; +import javax.xml.XMLConstants; import net.sf.freecol.common.FreeColException; import net.sf.freecol.common.debug.FreeColDebugger; @@ -101,6 +102,8 @@ protected Connection(String name) { Transformer myTransformer = null; try { TransformerFactory factory = TransformerFactory.newInstance(); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); myTransformer = factory.newTransformer(); myTransformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); diff --git a/src/net/sf/freecol/common/networking/DOMMessage.java b/src/net/sf/freecol/common/networking/DOMMessage.java index 7181a7d0bd..8fe7295443 100644 --- a/src/net/sf/freecol/common/networking/DOMMessage.java +++ b/src/net/sf/freecol/common/networking/DOMMessage.java @@ -37,6 +37,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; +import javax.xml.XMLConstants; import net.sf.freecol.common.io.FreeColXMLWriter; import net.sf.freecol.common.debug.FreeColDebugger; @@ -448,6 +449,8 @@ public static Element getChildElement(Element element, String tagName) { public static String elementToString(Element element) { try { TransformerFactory factory = TransformerFactory.newInstance(); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); Transformer xt = factory.newTransformer(); StringWriter sw = new StringWriter(); xt.transform(new DOMSource(element), new StreamResult(sw)); diff --git a/src/net/sf/freecol/tools/GenerateDocumentation.java b/src/net/sf/freecol/tools/GenerateDocumentation.java index aac0f55a4d..a52cf5bbae 100644 --- a/src/net/sf/freecol/tools/GenerateDocumentation.java +++ b/src/net/sf/freecol/tools/GenerateDocumentation.java @@ -35,6 +35,7 @@ import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; +import javax.xml.XMLConstants; import net.sf.freecol.common.i18n.Messages; import net.sf.freecol.common.model.StringTemplate; @@ -192,6 +193,8 @@ public static void generateDocumentation(String[] languages) { Messages.loadMessageBundle(Messages.getLocale(languageCode)); try { TransformerFactory factory = TransformerFactory.newInstance(); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); Source xsl = new StreamSource(new File("doc", XSL)); Transformer stylesheet; try {