diff --git a/hapi-base/src/main/java/ca/uhn/hl7v2/parser/XMLParser.java b/hapi-base/src/main/java/ca/uhn/hl7v2/parser/XMLParser.java index 428de4b0dc..9fa80eaa73 100644 --- a/hapi-base/src/main/java/ca/uhn/hl7v2/parser/XMLParser.java +++ b/hapi-base/src/main/java/ca/uhn/hl7v2/parser/XMLParser.java @@ -509,7 +509,7 @@ private void parseComposite(Composite datatypeObject, Element datatypeElement) compNum = Integer.parseInt(localName.substring(dotIndex + 1)) - 1; } else { log.debug( - "Datatype element {} doesn't have a valid numbered name, usgin default index of {}", + "Datatype element {} doesn't have a valid numbered name, using default index of {}", datatypeElement.getLocalName(), compNum); } Type nextComponent = datatypeObject.getComponent(compNum); diff --git a/hapi-base/src/main/java/ca/uhn/hl7v2/util/XMLUtils.java b/hapi-base/src/main/java/ca/uhn/hl7v2/util/XMLUtils.java index ce9e0f275e..7239615cbf 100644 --- a/hapi-base/src/main/java/ca/uhn/hl7v2/util/XMLUtils.java +++ b/hapi-base/src/main/java/ca/uhn/hl7v2/util/XMLUtils.java @@ -25,10 +25,10 @@ GNU General Public License (the "GPL"), in which case the provisions of the GPL */ package ca.uhn.hl7v2.util; -import java.io.InputStream; -import java.io.StringWriter; -import java.io.Writer; +import java.io.*; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.w3c.dom.DOMConfiguration; import org.w3c.dom.DOMErrorHandler; import org.w3c.dom.DOMImplementation; @@ -37,11 +37,17 @@ GNU General Public License (the "GPL"), in which case the provisions of the GPL import org.w3c.dom.ls.DOMImplementationLS; import org.w3c.dom.ls.LSInput; import org.w3c.dom.ls.LSOutput; -import org.w3c.dom.ls.LSParser; import org.w3c.dom.ls.LSResourceResolver; import org.w3c.dom.ls.LSSerializer; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; public class XMLUtils { + private static final Logger ourLog = LoggerFactory.getLogger(XMLUtils.class); private static DOMImplementation IMPL; @@ -59,43 +65,50 @@ public synchronized static T getDOMImpl() { } @SuppressWarnings("unchecked") - public static T getDOMImplUncached() { - try { - DOMImplementationRegistry registry = DOMImplementationRegistry.newInstance(); - return (T) registry.getDOMImplementation("LS 3.0"); - } catch (Exception e) { - throw new RuntimeException(e); - } - } - public static Document parse(String s) { - return parse(s, false); + return parseDocument(new InputSource(new StringReader(s)), false); } public static Document parse(String s, boolean validateIfSchema) { - DOMImplementationLS impl = getDOMImpl(); - LSInput input = impl.createLSInput(); - input.setStringData(s); - return parse(input, validateIfSchema); + return parseDocument(new InputSource(new StringReader(s)), validateIfSchema); } public static Document parse(InputStream s, boolean validateIfSchema) { - DOMImplementationLS impl = getDOMImpl(); - LSInput input = impl.createLSInput(); - input.setByteStream(s); - return parse(input, validateIfSchema); + return parseDocument(new InputSource(s), validateIfSchema); } - private static Document parse(LSInput input, boolean validateIfSchema) { - DOMImplementationLS impl = getDOMImpl(); - LSParser parser = impl.createLSParser(DOMImplementationLS.MODE_SYNCHRONOUS, null); - DOMConfiguration config = parser.getDomConfig(); - config.setParameter("element-content-whitespace", false); - config.setParameter("namespaces", true); - config.setParameter("validate-if-schema", validateIfSchema); - return parser.parse(input); + public static Document parseDocument(InputSource theInputSource, boolean theValidating) { + DocumentBuilder builder; + try { + DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); + docBuilderFactory.setNamespaceAware(true); + docBuilderFactory.setXIncludeAware(false); + docBuilderFactory.setExpandEntityReferences(false); + docBuilderFactory.setValidating(theValidating); + try { + docBuilderFactory.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", false); + docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); + docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + } catch (Exception e) { + ourLog.warn("Failed to set feature on XML parser: " + e.toString()); + } + + builder = docBuilderFactory.newDocumentBuilder(); + } catch (ParserConfigurationException e) { + throw new RuntimeException(e); + } + + try { + return builder.parse(theInputSource); + } catch (SAXException | IOException e) { + throw new RuntimeException(e); + } } + public static void validate(Document d, String schema, DOMErrorHandler handler) { DOMConfiguration config = d.getDomConfig(); config.setParameter("schema-type", "http://www.w3.org/2001/XMLSchema"); diff --git a/hapi-test/pom.xml b/hapi-test/pom.xml index 17a6d3693b..f4d5b7bc9b 100644 --- a/hapi-test/pom.xml +++ b/hapi-test/pom.xml @@ -195,9 +195,8 @@ org.mockito - mockito-all - ${mockito.version} - test + mockito-core + ${mockito_version} org.awaitility diff --git a/hapi-test/src/test/java/ca/uhn/hl7v2/app/ConnectionHubTest.java b/hapi-test/src/test/java/ca/uhn/hl7v2/app/ConnectionHubTest.java index 9408ee9d4d..c8d5aba433 100644 --- a/hapi-test/src/test/java/ca/uhn/hl7v2/app/ConnectionHubTest.java +++ b/hapi-test/src/test/java/ca/uhn/hl7v2/app/ConnectionHubTest.java @@ -36,11 +36,7 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; /** * JUnit test harmess for ConnectionHub connecting to a SimpleServer @@ -96,7 +92,7 @@ public void testConnectWithTlsSocketFactory() throws HL7Exception, IOException { HapiContext context = new DefaultHapiContext(); context.setValidationContext(ValidationContextFactory.noValidation()); - SocketFactory socketFactory = mock(SocketFactory.class); + SocketFactory socketFactory = mock(SocketFactory.class, withSettings().verboseLogging()); context.setSocketFactory(socketFactory); when(socketFactory.createTlsSocket()).thenReturn( javax.net.SocketFactory.getDefault().createSocket()); diff --git a/hapi-test/src/test/java/ca/uhn/hl7v2/parser/XMLParserTest.java b/hapi-test/src/test/java/ca/uhn/hl7v2/parser/XMLParserTest.java index 6a06f73a10..cf75b0a001 100644 --- a/hapi-test/src/test/java/ca/uhn/hl7v2/parser/XMLParserTest.java +++ b/hapi-test/src/test/java/ca/uhn/hl7v2/parser/XMLParserTest.java @@ -12,16 +12,14 @@ import static org.junit.Assert.assertTrue; // import hapi.on.olis.erp_z99.message.ERP_R09; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; +import java.io.*; import ca.uhn.hl7v2.DefaultHapiContext; import ca.uhn.hl7v2.HapiContext; import ca.uhn.hl7v2.Version; import ca.uhn.hl7v2.model.GenericMessage; import ca.uhn.hl7v2.util.Hl7InputStreamMessageIterator; +import ca.uhn.hl7v2.util.XMLUtils; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -39,6 +37,14 @@ import ca.uhn.hl7v2.model.v25.message.ORU_R01; import ca.uhn.hl7v2.model.v25.segment.OBX; import ca.uhn.hl7v2.util.Terser; +import org.w3c.dom.Node; + +import javax.xml.transform.OutputKeys; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.stream.StreamResult; /** * JUnit test harness for XMLParser @@ -467,6 +473,20 @@ public void testParseEncodedXml() throws Exception { } } + @Test + public void testParseRejextXxe() throws Exception { + String msg = loadFile("/ca/uhn/hl7v2/parser/xml-with-xxe.xml"); + + XMLParser p = DefaultXMLParser.getInstanceWithNoValidation(); + ADT_A01 a01 = (ADT_A01) p.parse(msg); + + ST msh10MessageControlID = a01.getMSH().getMsh10_MessageControlID(); + String value = msh10MessageControlID.getValue(); + ourLog.info("Encoded: {}", value); + assertEquals("", value); + + } + @Test public void testEncodeGenericMessage() throws Exception { DefaultXMLParser xmlParser = new DefaultXMLParser(); diff --git a/hapi-test/src/test/java/ca/uhn/hl7v2/protocol/impl/ApplicationRouterImplTest.java b/hapi-test/src/test/java/ca/uhn/hl7v2/protocol/impl/ApplicationRouterImplTest.java index 6ed12d3361..f54e3a2688 100644 --- a/hapi-test/src/test/java/ca/uhn/hl7v2/protocol/impl/ApplicationRouterImplTest.java +++ b/hapi-test/src/test/java/ca/uhn/hl7v2/protocol/impl/ApplicationRouterImplTest.java @@ -26,6 +26,7 @@ import ca.uhn.hl7v2.protocol.ReceivingApplicationExceptionHandler; import ca.uhn.hl7v2.protocol.Transportable; import ca.uhn.hl7v2.util.Terser; +import org.mockito.MockSettings; /** * Unit tests for ApplicationRouterImpl. @@ -74,17 +75,17 @@ public void testProcessMessage() throws Exception { @Test public void testExceptionHandlerWorksCorrectlyWithInvalidMessage() throws HL7Exception { - ReceivingApplicationExceptionHandler handler = mock(ReceivingApplicationExceptionHandler.class); + ReceivingApplicationExceptionHandler handler = mock(ReceivingApplicationExceptionHandler.class, withSettings().verboseLogging()); String msg = "BAD MESSAGE"; String respMsg = "BAD RESPONSE MESSAGE"; - when(handler.processException(eq(msg), any(Map.class), any(String.class), any(Exception.class))).thenReturn(respMsg); + when(handler.processException(eq(msg), any(), any(), any())).thenReturn(respMsg); myRouter.setExceptionHandler(handler); myRouter.processMessage(new TransportableImpl(msg)); - verify(handler).processException(eq(msg), any(Map.class), any(String.class), any(Exception.class)); + verify(handler).processException(eq(msg), any(), any(), any()); } @Test diff --git a/hapi-test/src/test/java/ca/uhn/hl7v2/validation/MessageValidatorTest.java b/hapi-test/src/test/java/ca/uhn/hl7v2/validation/MessageValidatorTest.java index bd8db68ae4..0c1a0eedb8 100644 --- a/hapi-test/src/test/java/ca/uhn/hl7v2/validation/MessageValidatorTest.java +++ b/hapi-test/src/test/java/ca/uhn/hl7v2/validation/MessageValidatorTest.java @@ -3,7 +3,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; diff --git a/hapi-test/src/test/java/ca/uhn/hl7v2/validation/RespondingValidationExceptionHandlerTest.java b/hapi-test/src/test/java/ca/uhn/hl7v2/validation/RespondingValidationExceptionHandlerTest.java index 08986315ba..3eb7af962a 100644 --- a/hapi-test/src/test/java/ca/uhn/hl7v2/validation/RespondingValidationExceptionHandlerTest.java +++ b/hapi-test/src/test/java/ca/uhn/hl7v2/validation/RespondingValidationExceptionHandlerTest.java @@ -1,7 +1,7 @@ package ca.uhn.hl7v2.validation; import static org.junit.Assert.assertEquals; -import static org.mockito.Matchers.any; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; diff --git a/hapi-test/src/test/resources/ca/uhn/hl7v2/parser/xml-with-xxe.xml b/hapi-test/src/test/resources/ca/uhn/hl7v2/parser/xml-with-xxe.xml new file mode 100644 index 0000000000..2d82e6f5e8 --- /dev/null +++ b/hapi-test/src/test/resources/ca/uhn/hl7v2/parser/xml-with-xxe.xml @@ -0,0 +1,20 @@ + + ]> + + + | + ^~\& + + ADT + A01 + ADT_A01 + + + P + + &xxe; + + 2.5 + + + \ No newline at end of file diff --git a/hapi-testmindeps/id_file b/hapi-testmindeps/id_file deleted file mode 100644 index 0454041723..0000000000 --- a/hapi-testmindeps/id_file +++ /dev/null @@ -1 +0,0 @@ -6600 diff --git a/pom.xml b/pom.xml index b0c1e2c6c3..42254badeb 100644 --- a/pom.xml +++ b/pom.xml @@ -644,7 +644,7 @@ 2.6 1.1.1 2.2 - 1.10.19 + 4.8.1 6.1.26 4.12 1.2.17 @@ -655,7 +655,6 @@ ${log4j.version} ${slf4j.version} ${hapi.version.stable} - ${project.parent.basedir} 3.3.0 diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 266e4b7c96..94b581b549 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -6,6 +6,12 @@ + + The XML parser was vulnerable to XXE injections (meaning that the XML + parser could be coerced into inadvertently loading the contents of files + on the local filesystem). It is highly recommended to upgrade to this + release in order to mitigate this issue. + As of HAPI 2.4, support for JDK 1.4 and JDK 1.5 has been withdrawn. It is no longer feasible to produce these binaries, as the retrotranslator plugin