diff --git a/src/main/java/org/apache/commons/validator/ValidatorResources.java b/src/main/java/org/apache/commons/validator/ValidatorResources.java index cf869ed4f..684cc2184 100644 --- a/src/main/java/org/apache/commons/validator/ValidatorResources.java +++ b/src/main/java/org/apache/commons/validator/ValidatorResources.java @@ -31,6 +31,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.xml.sax.Attributes; +import org.xml.sax.EntityResolver; +import org.xml.sax.InputSource; import org.xml.sax.SAXException; /** @@ -317,6 +319,29 @@ public void begin(final String namespace, final String name, final Attributes at } + /** + * EntityResolver that blocks remote external entity resolution. + */ + private static final class SecureEntityResolver implements EntityResolver { + + @Override + public InputSource resolveEntity(final String publicId, final String systemId) + throws SAXException, IOException { + if (publicId != null) { + return null; + } + if (systemId == null) { + return null; + } + final String lowerId = systemId.toLowerCase(Locale.ROOT); + if (lowerId.startsWith("http://") || lowerId.startsWith("https://") + || lowerId.startsWith("ftp://") || lowerId.startsWith("jar:")) { + throw new SAXException("Remote external entity resolution is disabled for system identifier: " + systemId); + } + return null; + } + } + /** * Add a {@code ValidatorAction} to the resource. It also creates an * instance of the class based on the {@code ValidatorAction}s @@ -585,6 +610,7 @@ private Digester initDigester() { digester.setNamespaceAware(true); digester.setValidating(true); digester.setUseContextClassLoader(true); + digester.setEntityResolver(new SecureEntityResolver()); // Add rules for arg0-arg3 elements addOldArgRules(digester); diff --git a/src/main/java/org/apache/commons/validator/util/ValidatorUtils.java b/src/main/java/org/apache/commons/validator/util/ValidatorUtils.java index 80ff198e3..2fed335a9 100644 --- a/src/main/java/org/apache/commons/validator/util/ValidatorUtils.java +++ b/src/main/java/org/apache/commons/validator/util/ValidatorUtils.java @@ -127,7 +127,7 @@ public static String getValueAsString(final Object bean, final String property) } if (value instanceof String[]) { - return ((String[]) value).length > 0 ? value.toString() : ""; + return ((String[]) value).length > 0 ? java.util.Arrays.toString((String[]) value) : ""; } if (value instanceof Collection) { diff --git a/src/test/java/org/apache/commons/validator/EntityImportTest.java b/src/test/java/org/apache/commons/validator/EntityImportTest.java index 622b283cb..306ab1964 100644 --- a/src/test/java/org/apache/commons/validator/EntityImportTest.java +++ b/src/test/java/org/apache/commons/validator/EntityImportTest.java @@ -16,12 +16,13 @@ */ package org.apache.commons.validator; -import static org.junit.jupiter.api.Assertions.assertNotNull; - import java.net.URL; import java.util.Locale; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import org.junit.jupiter.api.Test; +import org.xml.sax.SAXException; /** * Tests entity imports. @@ -47,4 +48,10 @@ void testParseURL() throws Exception { final ValidatorResources resources = new ValidatorResources(url); assertNotNull(resources.getForm(Locale.getDefault(), "byteForm"), "Form should be found"); } + + @Test + void testRemoteExternalEntityIsBlocked() throws Exception { + final URL url = getClass().getResource("ExternalEntityTest-config.xml"); + assertThrows(SAXException.class, () -> new ValidatorResources(url.toExternalForm())); + } } diff --git a/src/test/java/org/apache/commons/validator/util/ValidatorUtilsTest.java b/src/test/java/org/apache/commons/validator/util/ValidatorUtilsTest.java index f1cf5d233..6b7f5897b 100644 --- a/src/test/java/org/apache/commons/validator/util/ValidatorUtilsTest.java +++ b/src/test/java/org/apache/commons/validator/util/ValidatorUtilsTest.java @@ -19,6 +19,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.apache.commons.collections.FastHashMap; import org.junit.jupiter.api.Test; @@ -36,6 +42,25 @@ void testCopyFastHashMap() { original.setFast(true); final FastHashMap copy = ValidatorUtils.copyFastHashMap(original); assertEquals(original, copy); - } + } + + @Test + void testGetValueAsString() { + final Map map = new HashMap<>(); + map.put("string", "hello"); + map.put("emptyArray", new String[0]); + map.put("array", new String[]{"a", "b"}); + map.put("emptyCollection", Collections.emptyList()); + map.put("collection", Arrays.asList("a", "b")); + + assertEquals("hello", ValidatorUtils.getValueAsString(map, "string")); + assertEquals("", ValidatorUtils.getValueAsString(map, "emptyArray")); + assertEquals("", ValidatorUtils.getValueAsString(map, "emptyCollection")); + assertEquals("[a, b]", ValidatorUtils.getValueAsString(map, "collection")); + + final String arrayVal = ValidatorUtils.getValueAsString(map, "array"); + assertEquals("[a, b]", arrayVal); + } } + diff --git a/src/test/resources/org/apache/commons/validator/ExternalEntityTest-config.xml b/src/test/resources/org/apache/commons/validator/ExternalEntityTest-config.xml new file mode 100644 index 000000000..0906836ce --- /dev/null +++ b/src/test/resources/org/apache/commons/validator/ExternalEntityTest-config.xml @@ -0,0 +1,26 @@ + +]> + + + + &ext; + +