fix: an issue where encoded path traversal could bypass service ACL c…

src/common/common-core/src/main/java/ee/ria/xroad/common/util/UriUtils.java

@@ -25,6 +25,7 @@
  */
 package ee.ria.xroad.common.util;
 
+import java.net.URI;
 import java.nio.charset.StandardCharsets;
 
 /**
@@ -35,6 +36,16 @@ public final class UriUtils {
     private UriUtils() {
     }
 
+    /**
+     * Percent-decodes and normalizes a URI path, assuming UTF-8 character set.
+     *
+     * @see #uriPathPercentDecode(String, boolean)
+     */
+    public static String decodeAndNormalize(final String path) {
+        String decoded = uriPathPercentDecode(path, true);
+        return URI.create(decoded).normalize().getPath();
+    }
+
     /**
      * Percent-decodes a URI segment to a string, assuming UTF-8 character set.
      * The allowed chars in a URI segment are defined as follows:
@@ -45,6 +56,7 @@ private UriUtils() {
      * sub-delims    = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "="
      * pct-encoded   = "%" HEXDIG HEXDIG
      * </pre>
+     *
      * @see <a href="https://tools.ietf.org/html/rfc3986#section-3.3">RFC 3986</a>
      */
     public static String uriSegmentPercentDecode(final String src) {
@@ -53,6 +65,7 @@ public static String uriSegmentPercentDecode(final String src) {
 
     /**
      * Percent-decodes a URI path, assuming UTF-8 character set; optionally allows a path separator ('/').
+     *
      * @param src            URI path to percent-decode
      * @param allowSeparator If true, path separators are allowed and any %2d ('/') escape sequence is preserved
      *                       (normalized to %2D) so that it is possible to distinguish literal '/' from an encoded one.

src/common/common-core/src/test/java/ee/ria/xroad/common/util/UriUtilsTest.java

@@ -87,4 +87,18 @@ public void shouldKeepPathSeparator() {
     public void shouldFailIfPathSeparatorPresent() {
         uriPathPercentDecode("zy%2dggy/", false);
     }
+
+    @Test
+    public void shouldSuccessfullyNormalizeTraversalPaths() {
+        assertEquals("/a/b/c", UriUtils.decodeAndNormalize("/a/b/c"));
+        assertEquals("/a/b/c", UriUtils.decodeAndNormalize("/a/b/./c"));
+        assertEquals("/a/b/c", UriUtils.decodeAndNormalize("/a/b/d/../c"));
+        assertEquals("/c", UriUtils.decodeAndNormalize("/a/b/../../c"));
+        //Encoded variants
+        assertEquals("/a/b/c", UriUtils.decodeAndNormalize("/a/b/%2e/c"));
+        assertEquals("/a/b/c", UriUtils.decodeAndNormalize("/a/b/d/%2e%2e/c"));
+        assertEquals("/c", UriUtils.decodeAndNormalize("/a/b/%2e%2e/%2e%2e/c"));
+        assertEquals("/../c/d", UriUtils.decodeAndNormalize("/a/b/../../%2e%2e/c/d"));
+    }
+
 }

src/proxy/core/src/main/java/ee/ria/xroad/proxy/util/MessageProcessorBase.java

@@ -183,7 +183,8 @@ protected void updateOpMonitoringDataByRestRequest(OpMonitoringData opMonitoring
     }
 
     private String getNormalizedServicePath(String servicePath) {
-        return Optional.of(UriUtils.uriPathPercentDecode(URI.create(servicePath).normalize().getRawPath(), true))
+        return Optional.ofNullable(servicePath)
+                .map(UriUtils::decodeAndNormalize)
                 .orElse(servicePath);
     }
 

src/serverconf/src/main/java/ee/ria/xroad/common/conf/serverconf/ServerConfImpl.java

@@ -70,7 +70,6 @@
 import org.hibernate.Session;
 import org.hibernate.SharedSessionContract;
 
-import java.net.URI;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.List;
@@ -422,7 +421,13 @@ private boolean checkAccessRights(Session session, ClientId client, ServiceId se
         if (path == null) {
             normalizedPath = null;
         } else {
-            normalizedPath = UriUtils.uriPathPercentDecode(URI.create(path).normalize().getRawPath(), true);
+            normalizedPath = UriUtils.decodeAndNormalize(path);
+
+            // Explicitly reject any remaining traversal sequences
+            if (normalizedPath.contains("..")) {
+                log.warn("Path traversal detected in request path: {}. Access will be rejected.", path);
+                return false;
+            }
         }
         return getAclEndpoints(session, client, service).stream()
                 .anyMatch(ep -> ep.matches(method, normalizedPath));

src/serverconf/src/test/java/ee/ria/xroad/proxy/conf/CachingServerConfTest.java

@@ -255,6 +255,10 @@ public void isQueryAllowed() {
         assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "POST", "/api/test/foo/bar"));
         assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "DELETE", "/api/test"));
         assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest));
+
+        assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "GET", "/%2e%2e/secret"));
+        assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "GET", "/api/%2e%2e/secret"));
+        assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "GET", "/api/test/%2e%2e/%2e%2e/secret"));
     }
 
     /**

src/serverconf/src/test/java/ee/ria/xroad/proxy/conf/ServerConfTest.java

@@ -250,6 +250,10 @@ public void isQueryAllowed() {
         assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "POST", "/api/test/foo/bar"));
         assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "DELETE", "/api/test"));
         assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest));
+
+        assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "GET", "/%2e%2e/secret"));
+        assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "GET", "/api/%2e%2e/secret"));
+        assertFalse(serverConfProvider.isQueryAllowed(client1, serviceRest, "GET", "/api/test/%2e%2e/%2e%2e/secret"));
     }
 
     /**