Prevent URL path traversal bypass via percent encoding in UrlValidator

src/main/java/org/apache/commons/validator/routines/UrlValidator.java

@@ -17,8 +17,11 @@
 package org.apache.commons.validator.routines;
 
 import java.io.Serializable;
+import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Locale;
@@ -29,10 +32,8 @@
 import org.apache.commons.validator.GenericValidator;
 
 /**
- * <strong>URL Validation</strong> routines.
- * <p>
+ * <p><strong>URL Validation</strong> routines.</p>
  * Behavior of validation is modified by passing in options:
- * </p>
  * <ul>
  * <li>ALLOW_2_SLASHES - [FALSE]  Allows double '/' characters in the path
  * component.</li>
@@ -356,7 +357,7 @@ private boolean isOn(final long flag) {
     }
 
     /**
-     * Checks if a field has a valid URL address.
+     * <p>Checks if a field has a valid URL address.</p>
      *
      * Note that the method calls #isValidAuthority()
      * which checks that the domain is valid.
@@ -488,19 +489,19 @@ protected boolean isValidPath(final String path) {
         }
 
         try {
+            final String decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8.name());
             // Don't omit host otherwise leading path may be taken as host if it starts with //
-            final URI uri = new URI(null, "localhost", path, null);
+            final URI uri = new URI(null, "localhost", decodedPath, null);
             final String norm = uri.normalize().getPath();
             if (norm.startsWith("/../") // Trying to go via the parent dir
                     || norm.equals("/..")) { // Trying to go to the parent dir
                 return false;
             }
-        } catch (final URISyntaxException e) {
-            return false;
-        }
-
-        final int slash2Count = countToken("//", path);
-        if (isOff(ALLOW_2_SLASHES) && slash2Count > 0) {
+            final int slash2Count = countToken("//", decodedPath);
+            if (isOff(ALLOW_2_SLASHES) && slash2Count > 0) {
+                return false;
+            }
+        } catch (final UnsupportedEncodingException | IllegalArgumentException | URISyntaxException e) {
             return false;
         }
 

src/test/java/org/apache/commons/validator/routines/UrlValidatorTest.java

@@ -626,4 +626,41 @@ void testValidator473Part3() { // reject null DomainValidator with mismatched al
         assertEquals("DomainValidator disagrees with ALLOW_LOCAL_URLS setting", thrown.getMessage());
     }
 
+    @Test
+    void testValidator383() {
+        final UrlValidator validator = new UrlValidator();
+
+        // Literal traversal checks (already rejected)
+        assertFalse(validator.isValid("http://example.com/../etc/passwd"));
+        assertFalse(validator.isValid("http://example.com/.."));
+        assertFalse(validator.isValid("http://example.com/../"));
+
+        // Percent-encoded traversal checks
+        assertFalse(validator.isValid("http://example.com/..%2fetc/passwd"));
+        assertFalse(validator.isValid("http://example.com/..%2Fetc/passwd"));
+        assertFalse(validator.isValid("http://example.com/%2e%2e/world"));
+        assertFalse(validator.isValid("http://example.com/%2e%2e%2fworld"));
+        assertFalse(validator.isValid("http://example.com/%2E%2e%2Fworld"));
+
+        // Consecutive slashes via percent encoding
+        final UrlValidator noDoubleSlashes = new UrlValidator();
+        assertFalse(noDoubleSlashes.isValid("http://example.com/foo%2F%2Fbar"));
+        assertFalse(noDoubleSlashes.isValid("http://example.com/foo%2f%2fbar"));
+        assertFalse(noDoubleSlashes.isValid("http://example.com/%2F%2Fbar"));
+
+        final UrlValidator allowDoubleSlashes = new UrlValidator(UrlValidator.ALLOW_2_SLASHES);
+        assertTrue(allowDoubleSlashes.isValid("http://example.com/foo%2F%2Fbar"));
+        assertTrue(allowDoubleSlashes.isValid("http://example.com/foo%2f%2fbar"));
+        assertTrue(allowDoubleSlashes.isValid("http://example.com/%2F%2Fbar"));
+
+        // Invalid percent-encoding handling
+        assertFalse(validator.isValid("http://example.com/foo%2"));
+        assertFalse(validator.isValid("http://example.com/foo%2G"));
+        assertFalse(validator.isValid("http://example.com/foo%"));
+
+        // Plus character preservation
+        assertTrue(validator.isValid("http://example.com/foo+bar"));
+        assertTrue(validator.isValid("http://example.com/foo+bar/baz+qux"));
+    }
+
 }