Eclipse Jetty: Path parameter traversal

Description (as reported)

Summary

In Jetty 12.1.8, org.eclipse.jetty.util.URIUtil.canonicalPath() may leave dot-dot path segments unnormalized when a semicolon path parameter marker is followed by a slash and a dot
segment.

A minimal example is:

/public;/../admin/secret

In my local reproduction, URIUtil.canonicalPath() returns:

/public/../admin/secret

instead of the expected normalized path:

/admin/secret

When Jetty's SecurityHandler.PathMapped is used to protect a path prefix such as /admin/*, the non-normalized canonical path may not match the protected prefix. As a result, an unauthenticated request may bypass the configured path-based security constraint.

Tested Version

Jetty: 12.1.8
JDK: 17.0.18
Maven: 3.9.14

Maven artifacts used:

org.eclipse.jetty:jetty-server:12.1.8
org.eclipse.jetty:jetty-security:12.1.8
org.eclipse.jetty:jetty-session:12.1.8

Only confirmed Jetty 12.1.8 so far.

Minimal Reproduction

Starts a minimal Jetty server with the following security setup:

 1SecurityHandler.PathMapped security = new SecurityHandler.PathMapped();
 2security.put("/admin/*", Constraint.from("admin"));
 3security.put("/*", Constraint.ALLOWED);
 4security.setAuthenticator(new BasicAuthenticator());

The test then sends requests with no Authorization header.

Observed result:

 1GET /admin/secret                  -> 401
 2GET /public;x/../admin/secret      -> 200

The handler receives paths such as:

/public/../admin/secret

This suggests that the /admin/* security constraint is bypassed because PathMapped matching is performed against the non-normalized canonical path.

Suspected Root Cause

The suspected root cause is in URIUtil.canonicalPath().

The relevant logic is approximately:

 1    for (int i = 0; i < end; i++)
 2    {
 3        char c = encodedPath.charAt(i);
 4 
 5        switch (c)
 6        {
 7            case ';':
 8                if (builder == null)
 9                {
10                    builder = new Utf8StringBuilder(encodedPath.length());
11                    builder.append(encodedPath, 0, i);
12                }
13 
14                while (++i < end)
15                {
16                    if (encodedPath.charAt(i) == '/')
17                    {
18                        builder.append('/');
19                        break;
20                    }
21                }
22                break;
23 
24            case '.':
25                if (slash)
26                    normal = false;
27                if (builder != null)
28                    builder.append(c);
29                break;
30        }
31 
32        slash = c == '/';
33    }
34 
35    String canonical = (builder != null)
36        ? (onBadUtf8 == null ? builder.toCompleteString() : builder.takeCompleteString(onBadUtf8))
37        : encodedPath;
38    return normal ? canonical : normalizePath(canonical);

For the input:

/public;/../admin/secret

when the outer loop reaches the semicolon:

 1    i      = 7
 2    c      = ';'
 3    slash  = false
 4    normal = true

Inside case ';', the while (++i < end) loop advances i to the next character, which is already '/' for the empty path parameter form ";/".

The code then appends '/' to the canonical builder:

builder.append('/');

At this point, the canonical builder ends with '/':

/public/

However, the local variable c is still the old value ';', because c was read before entering the switch and is not updated when the inner loop advances i.

After leaving the switch, the loop updates the slash state using:

slash = c == '/';

Since c is still ';', slash becomes false.

On the next iteration, the scanner reaches '.', which is the first dot in the following "../" segment. Because slash is incorrectly false, this code does not run:

 1    if (slash)
 2        normal = false;

Therefore normal remains true, and canonicalPath() returns the canonical string directly instead of calling normalizePath(canonical).

The result is:

/public/../admin/secret

instead of:

/admin/secret

In short:

case ';' advances the scan position i and appends '/' to the canonical builder, but the loop tail still updates slash from the stale character c=';'. As a result, the following dot-dot segment is not detected as a path traversal segment.

More Precise Trigger Condition

The issue is not limited to a non-empty path parameter such as ";x".

The more precise trigger shape is:

;[^/]*/.

Examples:

 1    /public;/../admin/secret
 2    /public;x/../admin/secret
 3    /public;anything/../admin/secret
 4    /public;/./admin/secret

The minimal form is:

/public;/../admin/secret

because the semicolon is immediately followed by '/', so the inner while loop reaches '/' on its first increment.

Potential Minimal Fix Direction

A minimal fix would be to ensure that, when case ';' consumes input until '/' and appends '/' to the canonical builder, the slash state reflects the last effective character in the canonical path.

For example, conceptually:

 1    case ';':
 2        if (builder == null)
 3        {
 4            builder = new Utf8StringBuilder(encodedPath.length());
 5            builder.append(encodedPath, 0, i);
 6        }
 7 
 8        while (++i < end)
 9        {
10            if (encodedPath.charAt(i) == '/')
11            {
12                builder.append('/');
13                slash = true;
14                break;
15            }
16        }
17        continue;

The important part is to avoid the loop tail from overwriting slash using the stale c value:

slash = c == '/';

In other words, slash should represent the last effective character appended to the canonical builder, not the original input character read before case ';' advanced i.