Skip to content

Commit

Permalink
[7.x~DROOLS-6810 Correct fix for potential XXE vulnerabilities (apach…
Browse files Browse the repository at this point in the history 
…e#4313)

* DROOLS-6810 Correct fix for potential XXE vulnerabilities

* Fixed typo

* Minor fixes
  • Loading branch information
@pibizza
pibizza committed Apr 19, 2022
1 parent f2fb385 commit dc0c474
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 41 deletions.
35 changes: 25 additions & 10 deletions ...ation-backend/src/main/java/org/drools/scenariosimulation/backend/util/DOMParserUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.OutputKeys;
import javax.xml.transform.Source;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.TransformerFactoryConfigurationError;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;

Expand Down Expand Up @@ -340,28 +343,40 @@ public static List<Node> getNestedChildrenNodesList(Node node, String containerN
}

public static Document getDocument(String xml) throws ParserConfigurationException, IOException, SAXException {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
DocumentBuilder dBuilder = factory.newDocumentBuilder();
DocumentBuilder dBuilder = createBuilder();
try (InputStream inputStream = new ByteArrayInputStream(xml.getBytes())) {
return dBuilder.parse(inputStream);
}
}

public static String getString(Document toRead) throws TransformerException {
DOMSource domSource = new DOMSource(toRead);
TransformerFactory factory = TransformerFactory.newInstance();
static DocumentBuilder createBuilder() throws ParserConfigurationException {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance("com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl", null);
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // REQUIRED - prevents XXE attack
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // REQUIRED - prevents XXE attack
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Transformer transformer = factory.newTransformer();
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2");
return factory.newDocumentBuilder();
}

public static String getString(Document toRead) throws TransformerException {
Source domSource = new DOMSource(toRead);
Transformer transformer = createTransformer();
StringWriter sw = new StringWriter();
StreamResult sr = new StreamResult(sw);
transformer.transform(domSource, sr);
return sw.toString();
}

static Transformer createTransformer() throws TransformerFactoryConfigurationError, TransformerConfigurationException {
TransformerFactory factory = TransformerFactory.newInstance("com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl", null);
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // required to keep SonarCloud quiet
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); // required to keep SonarCloud quiet
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Transformer transformer = factory.newTransformer();
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2");
return transformer;
}

/**
* Recursively populate the given <code>Map</code>
* @param node
Expand Down
39 changes: 39 additions & 0 deletions ...n-backend/src/test/java/org/drools/scenariosimulation/backend/util/DOMParserUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.drools.scenariosimulation.backend.util;

import java.io.ByteArrayInputStream;
import java.io.StringWriter;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand All @@ -23,6 +25,10 @@

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.transform.Transformer;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import javax.xml.transform.stream.StreamSource;

import org.junit.Test;
import org.w3c.dom.Document;
Expand Down Expand Up @@ -443,6 +449,23 @@ public void getDocument() throws Exception {
assertNotNull(retrieved);
}



@Test(expected = org.xml.sax.SAXParseException.class)
public void getDocument_XXE_Vulnerability() throws Exception {
String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<!DOCTYPE foo [ <!ENTITY xxe SYSTEM \"file:///\"> ]>\n"
+ "<stocklevel><ProductID>&xxe;</ProductID></stocklevel>";
Document retrieved = DOMParserUtil.getDocument(xml);

Transformer transformer = DOMParserUtil.createTransformer();

StringWriter sw = new StringWriter();
StreamResult sr = new StreamResult(sw);
transformer.transform(new DOMSource(retrieved), sr);
}


@Test
public void getString() throws Exception {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
Expand All @@ -453,6 +476,22 @@ public void getString() throws Exception {
assertNotNull(retrieved);
assertTrue(retrieved.contains("CREATED"));
}

@Test(expected = javax.xml.transform.TransformerException.class)
public void createTransformer_XXE_Vulnerability() throws Exception {
String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<!DOCTYPE foo [ <!ENTITY xxe SYSTEM \"file:///\"> ]>\n"
+ "<stocklevel><ProductID>&xxe;</ProductID></stocklevel>";
StreamSource source = new StreamSource(new ByteArrayInputStream(xml.getBytes()));

Transformer transformer = DOMParserUtil.createTransformer();

StringWriter sw = new StringWriter();
StreamResult sr = new StreamResult(sw);
transformer.transform(source, sr);
}



@Test
public void asStream() throws Exception {
Expand Down
32 changes: 1 addition & 31 deletions ...backend/src/main/java/org/kie/dmn/backend/marshalling/v1_1/xstream/XStreamMarshaller.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package org.kie.dmn.backend.marshalling.v1_1.xstream;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Reader;
Expand All @@ -28,18 +26,10 @@
import java.util.List;
import java.util.Map;

import javax.xml.XMLConstants;
import javax.xml.namespace.QName;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
import javax.xml.stream.XMLStreamWriter;
import javax.xml.transform.OutputKeys;
import javax.xml.transform.Source;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.sax.SAXSource;
import javax.xml.transform.sax.SAXTransformerFactory;
import javax.xml.transform.stream.StreamResult;

import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.io.xml.AbstractPullReader;
Expand Down Expand Up @@ -94,7 +84,6 @@
import org.kie.soup.xstream.XStreamUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xml.sax.InputSource;

public class XStreamMarshaller
implements DMNMarshaller {
Expand Down Expand Up @@ -208,26 +197,7 @@ public void marshalMarshall(Object o, OutputStream out) {
e.printStackTrace();
}
}

/**
* Unnecessary as the stax driver custom anon as static definition is embedding the indentation.
*/
@Deprecated
public static String formatXml(String xml){
try{
TransformerFactory transformerFactory = SAXTransformerFactory.newInstance();
transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
Transformer serializer = transformerFactory.newTransformer();
serializer.setOutputProperty(OutputKeys.INDENT, "yes");
serializer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2");
Source xmlSource=new SAXSource(new InputSource(new ByteArrayInputStream(xml.getBytes())));
StreamResult res = new StreamResult(new ByteArrayOutputStream());
serializer.transform(xmlSource, res);
return new String(((ByteArrayOutputStream)res.getOutputStream()).toByteArray());
}catch(Exception e){
return xml;
}
}


private XStream newXStream() {
XStream xStream = XStreamUtils.createNonTrustingXStream(staxDriver, Definitions.class.getClassLoader(), DMNXStream::from);
Expand Down

0 comments on commit dc0c474

Please sign in to comment.