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/src/main/java/ee/ria/xroad/proxy/util/MessageProcessorBase.java

@@ -53,13 +53,19 @@
 @Slf4j
 public abstract class MessageProcessorBase {
 
-    /** The servlet request. */
+    /**
+     * The servlet request.
+     */
     protected final RequestWrapper jRequest;
 
-    /** The servlet response. */
+    /**
+     * The servlet response.
+     */
     protected final ResponseWrapper jResponse;
 
-    /** The http client instance. */
+    /**
+     * The http client instance.
+     */
     protected final HttpClient httpClient;
 
     protected MessageProcessorBase(RequestWrapper request,
@@ -93,13 +99,15 @@ protected void postprocess() throws Exception {
 
     /**
      * Processes the incoming message.
+     *
      * @throws Exception in case of any errors
      */
     public abstract void process() throws Exception;
 
     /**
      * Update operational monitoring data with SOAP message header data and
      * the size of the message.
+     *
      * @param opMonitoringData monitoring data to update
      * @param soapMessage      SOAP message
      */
@@ -152,6 +160,7 @@ protected static String getSecurityServerAddress() {
      * Validates SOAPAction header value.
      * Valid header values are: (empty string),(""),("URI-reference")
      * In addition, this implementation allows missing (null) header.
+     *
      * @return the argument as-is if it is valid
      * @throws CodedException if the the argument is invalid
      * @see <a href="https://www.w3.org/TR/2000/NOTE-SOAP-20000508/#_Toc478383528">SOAP 1.1</a>
@@ -177,6 +186,7 @@ protected static String validateSoapActionHeader(String soapAction) {
 
     /**
      * Logs a warning if identifier contains invalid characters.
+     *
      * @see ee.ria.xroad.common.validation.SpringFirewallValidationRules
      * @see ee.ria.xroad.common.validation.LegacyEncodedIdentifierValidator;
      */

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

@@ -67,7 +67,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;
@@ -410,7 +409,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

@@ -87,6 +87,7 @@ public class CachingServerConfTest {
 
     /**
      * Creates test database.
+     *
      * @throws Exception if an error occurs
      */
     @BeforeClass
@@ -250,6 +251,10 @@ public void isQueryAllowed() {
         assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "POST", "/api/test/foo/bar"));
         assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "DELETE", "/api/test"));
         assertFalse(ServerConf.isQueryAllowed(client1, serviceRest));
+
+        assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "GET", "/%2e%2e/secret"));
+        assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "GET", "/api/%2e%2e/secret"));
+        assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "GET", "/api/test/%2e%2e/%2e%2e/secret"));
     }
 
     /**
@@ -267,6 +272,7 @@ public void getIsAuthentication() {
 
     /**
      * Tests getting IS certificates,
+     *
      * @throws Exception if an error coccurs
      */
     @Test
@@ -297,6 +303,7 @@ public void isSslAuthentication() {
 
     /**
      * Tests getting members.
+     *
      * @throws Exception if an error coccurs
      */
     @Test
@@ -308,6 +315,7 @@ public void getMembers() throws Exception {
 
     /**
      * Tests getting TSPs.
+     *
      * @throws Exception if an error occurs
      */
     @Test
@@ -318,6 +326,7 @@ public void getTsps() throws Exception {
 
     /**
      * Tests getting services.
+     *
      * @throws Exception if an error occurs
      */
     @Test

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

@@ -86,6 +86,7 @@ public class ServerConfTest {
 
     /**
      * Creates test database.
+     *
      * @throws Exception if an error occurs
      */
     @BeforeClass
@@ -243,6 +244,10 @@ public void isQueryAllowed() {
         assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "POST", "/api/test/foo/bar"));
         assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "DELETE", "/api/test"));
         assertFalse(ServerConf.isQueryAllowed(client1, serviceRest));
+
+        assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "GET", "/%2e%2e/secret"));
+        assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "GET", "/api/%2e%2e/secret"));
+        assertFalse(ServerConf.isQueryAllowed(client1, serviceRest, "GET", "/api/test/%2e%2e/%2e%2e/secret"));
     }
 
     /**
@@ -274,6 +279,7 @@ public void getIsAuthentication() {
 
     /**
      * Tests getting IS certificates,
+     *
      * @throws Exception if an error coccurs
      */
     @Test
@@ -304,6 +310,7 @@ public void isSslAuthentication() {
 
     /**
      * Tests getting members.
+     *
      * @throws Exception if an error coccurs
      */
     @Test
@@ -315,6 +322,7 @@ public void getMembers() throws Exception {
 
     /**
      * Tests getting TSPs.
+     *
      * @throws Exception if an error occurs
      */
     @Test
@@ -328,6 +336,7 @@ public void getTsps() throws Exception {
 
     /**
      * Tests getting services.
+     *
      * @throws Exception if an error occurs
      */
     @Test