From 7974f55641f42d6fc95b014307534f66c9ce8994 Mon Sep 17 00:00:00 2001 From: Kostiantyn Severynov Date: Wed, 5 Jan 2022 23:58:47 +0200 Subject: [PATCH 1/2] Fix XXE vulnerability --- jpos/build.gradle | 1 + jpos/libraries.gradle | 7 ++++- .../jpos/iso/packager/GenericPackager.java | 3 ++ .../packager/GenericValidatingPackager.java | 3 ++ .../iso/packager/GenericPackagerTest.java | 28 ++++++++++++++---- .../GenericValidatingPackagerTest.java | 29 +++++++++++++++---- 6 files changed, 60 insertions(+), 11 deletions(-) diff --git a/jpos/build.gradle b/jpos/build.gradle index f54b955074..fbe80329ac 100644 --- a/jpos/build.gradle +++ b/jpos/build.gradle @@ -39,6 +39,7 @@ dependencies { exclude(module: 'hamcrest-core') } testImplementation libraries.mockito_jupiter + testImplementation libraries.mockserver // JSONPackager on hold // compile (libraries.jsonsimple) { diff --git a/jpos/libraries.gradle b/jpos/libraries.gradle index 2819c37314..84a718530f 100644 --- a/jpos/libraries.gradle +++ b/jpos/libraries.gradle @@ -11,6 +11,10 @@ ext { javassist: 'org.javassist:javassist:3.27.0-GA', osgi_core: 'org.osgi:org.osgi.core:6.0.0', mockito: 'org.mockito:mockito-core:3.8.0', + mockserver: [ + 'org.mock-server:mockserver-netty:5.11.2', + 'org.mock-server:mockserver-client-java:5.11.2' + ], mockito_jupiter: 'org.mockito:mockito-junit-jupiter:3.8.0', fest_assert: 'org.easytesting:fest-assert:1.4', xmlunit: 'xmlunit:xmlunit:1.6', @@ -25,7 +29,8 @@ ext { slf4j_api: "org.slf4j:slf4j-api:1.7.32", slf4j_nop: "org.slf4j:slf4j-nop:1.7.32", hdrhistogram: 'org.hdrhistogram:HdrHistogram:2.1.12', - yaml: "org.yaml:snakeyaml:1.28" + yaml: "org.yaml:snakeyaml:1.28", + ] } diff --git a/jpos/src/main/java/org/jpos/iso/packager/GenericPackager.java b/jpos/src/main/java/org/jpos/iso/packager/GenericPackager.java index 52a9c150dc..4fbd584cf8 100644 --- a/jpos/src/main/java/org/jpos/iso/packager/GenericPackager.java +++ b/jpos/src/main/java/org/jpos/iso/packager/GenericPackager.java @@ -251,6 +251,9 @@ private XMLReader createXMLReader () throws SAXException { ); } reader.setFeature ("http://xml.org/sax/features/validation", true); + reader.setFeature("http://xml.org/sax/features/external-general-entities", false); + reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + reader.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); GenericContentHandler handler = new GenericContentHandler(); reader.setContentHandler(handler); reader.setErrorHandler(handler); diff --git a/jpos/src/main/java/org/jpos/iso/packager/GenericValidatingPackager.java b/jpos/src/main/java/org/jpos/iso/packager/GenericValidatingPackager.java index 2cd9638e78..0f8ab56bfb 100644 --- a/jpos/src/main/java/org/jpos/iso/packager/GenericValidatingPackager.java +++ b/jpos/src/main/java/org/jpos/iso/packager/GenericValidatingPackager.java @@ -102,6 +102,9 @@ public void readFile(String filename) throws org.jpos.iso.ISOException { System.getProperty( "sax.parser", "org.apache.crimson.parser.XMLReaderImpl")); reader.setFeature ("http://xml.org/sax/features/validation", true); + reader.setFeature("http://xml.org/sax/features/external-general-entities", false); + reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + reader.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); GenericValidatorContentHandler handler = new GenericValidatorContentHandler(); reader.setContentHandler(handler); reader.setErrorHandler(handler); diff --git a/jpos/src/test/java/org/jpos/iso/packager/GenericPackagerTest.java b/jpos/src/test/java/org/jpos/iso/packager/GenericPackagerTest.java index b24297ecde..490549a822 100644 --- a/jpos/src/test/java/org/jpos/iso/packager/GenericPackagerTest.java +++ b/jpos/src/test/java/org/jpos/iso/packager/GenericPackagerTest.java @@ -18,20 +18,20 @@ package org.jpos.iso.packager; +import static org.apache.commons.lang3.JavaVersion.JAVA_10; +import static org.apache.commons.lang3.JavaVersion.JAVA_14; +import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtMost; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; - -import static org.apache.commons.lang3.JavaVersion.JAVA_10; -import static org.apache.commons.lang3.JavaVersion.JAVA_14; -import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtMost; +import static org.mockserver.integration.ClientAndServer.startClientAndServer; +import static org.mockserver.model.HttpRequest.request; import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.util.EmptyStackException; - import org.jpos.core.Configuration; import org.jpos.core.ConfigurationException; import org.jpos.core.SimpleConfiguration; @@ -42,6 +42,8 @@ import org.jpos.iso.ISOFieldPackager; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.mockserver.integration.ClientAndServer; +import org.mockserver.verify.VerificationTimes; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; @@ -550,4 +552,20 @@ public void testSetConfigurationThrowsNullPointerException() throws Throwable { assertNull(genericSubFieldPackager.getRealm(), "(GenericSubFieldPackager) genericSubFieldPackager.getRealm()"); } } + + @Test + public void testXXEAttach() throws ISOException { + ClientAndServer mockServer = startClientAndServer(1080); + + String xeeAttackXml = "\n" + + "\n" + + " ]>\n" + + "&xxe;"; + new GenericPackager().readFile(new ByteArrayInputStream(xeeAttackXml.getBytes())); + mockServer.verify( + request().withPath("/xxe"), + VerificationTimes.exactly(0) + ); + } } diff --git a/jpos/src/test/java/org/jpos/iso/packager/GenericValidatingPackagerTest.java b/jpos/src/test/java/org/jpos/iso/packager/GenericValidatingPackagerTest.java index ec2eb2162a..7993dc6071 100644 --- a/jpos/src/test/java/org/jpos/iso/packager/GenericValidatingPackagerTest.java +++ b/jpos/src/test/java/org/jpos/iso/packager/GenericValidatingPackagerTest.java @@ -18,6 +18,9 @@ package org.jpos.iso.packager; +import static org.apache.commons.lang3.JavaVersion.JAVA_10; +import static org.apache.commons.lang3.JavaVersion.JAVA_14; +import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtMost; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -25,10 +28,8 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; - -import static org.apache.commons.lang3.JavaVersion.JAVA_10; -import static org.apache.commons.lang3.JavaVersion.JAVA_14; -import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtMost; +import static org.mockserver.integration.ClientAndServer.startClientAndServer; +import static org.mockserver.model.HttpRequest.request; import java.io.ByteArrayInputStream; import java.util.ArrayList; @@ -36,7 +37,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; - import org.jpos.iso.ISOBaseValidator; import org.jpos.iso.ISOException; import org.jpos.iso.ISOFieldValidator; @@ -51,6 +51,8 @@ import org.jpos.iso.validator.TEST0100; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.mockserver.integration.ClientAndServer; +import org.mockserver.verify.VerificationTimes; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; @@ -871,4 +873,21 @@ public void testValidateThrowsNullPointerException4() throws Throwable { } } } + + @Test + public void testXXEAttach() throws ISOException { + ClientAndServer mockServer = startClientAndServer(1081); + + String xeeAttackXml = "\n" + + "\n" + + " ]>\n" + + "&xxe;"; + new GenericValidatingPackager().readFile(new ByteArrayInputStream(xeeAttackXml.getBytes())); + + mockServer.verify( + request().withPath("/xxe"), + VerificationTimes.exactly(0) + ); + } } From d1be2db19b21355ae6f223ac7620b8e8d5df5918 Mon Sep 17 00:00:00 2001 From: Kostiantyn Severynov Date: Thu, 6 Jan 2022 00:02:14 +0200 Subject: [PATCH 2/2] update libs --- jpos/libraries.gradle | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jpos/libraries.gradle b/jpos/libraries.gradle index 84a718530f..2ce2f4fbd2 100644 --- a/jpos/libraries.gradle +++ b/jpos/libraries.gradle @@ -11,10 +11,6 @@ ext { javassist: 'org.javassist:javassist:3.27.0-GA', osgi_core: 'org.osgi:org.osgi.core:6.0.0', mockito: 'org.mockito:mockito-core:3.8.0', - mockserver: [ - 'org.mock-server:mockserver-netty:5.11.2', - 'org.mock-server:mockserver-client-java:5.11.2' - ], mockito_jupiter: 'org.mockito:mockito-junit-jupiter:3.8.0', fest_assert: 'org.easytesting:fest-assert:1.4', xmlunit: 'xmlunit:xmlunit:1.6', @@ -30,7 +26,10 @@ ext { slf4j_nop: "org.slf4j:slf4j-nop:1.7.32", hdrhistogram: 'org.hdrhistogram:HdrHistogram:2.1.12', yaml: "org.yaml:snakeyaml:1.28", - + mockserver: [ + 'org.mock-server:mockserver-netty:5.11.2', + 'org.mock-server:mockserver-client-java:5.11.2' + ] ] }