Skip to content

Commit

Permalink
Improve #4275 ambiguous URIs (#6939)
Browse files Browse the repository at this point in the history
* Improve #4275 ambiguous URIs

A URI like `/foo/%2e%2e;/bar` should be ambiguous both because of the encoded dots and because of the parameters.  This means that the default setting of jetty-9 is a bit more secure as this path is considered ambiguous if either Violation.SEGMENT or Violation.PARAM is set.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
lachlan-roberts committed Oct 12, 2021
1 parent ca8d147 commit 866f451
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
11 changes: 5 additions & 6 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
Expand Up @@ -1414,15 +1414,14 @@ private void checkSegment(String uri, int segment, int end, boolean param)

// Look for segment in the ambiguous segment index.
Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment);
if (ambiguous == Boolean.TRUE)
if (ambiguous != null)
{
// The segment is always ambiguous.
_violations.add(Violation.AMBIGUOUS_PATH_SEGMENT);
}
else if (param && ambiguous == Boolean.FALSE)
{
if (Boolean.TRUE.equals(ambiguous))
_violations.add(Violation.AMBIGUOUS_PATH_SEGMENT);
// The segment is ambiguous only when followed by a parameter.
_violations.add(Violation.AMBIGUOUS_PATH_PARAMETER);
if (param)
_violations.add(Violation.AMBIGUOUS_PATH_PARAMETER);
}
}
}
Expand Down
Expand Up @@ -59,7 +59,7 @@ public enum Violation implements ComplianceViolation
*/
AMBIGUOUS_PATH_SEPARATOR("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path separator"),
/**
* Allow ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
* Allow ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code> or <code>/foo/%2e%2e;param/bar</code>
*/
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter"),
/**
Expand Down
12 changes: 6 additions & 6 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
Expand Up @@ -395,9 +395,9 @@ public static Stream<Arguments> decodePathTests()
{"/path/%2e/info", "/path/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"path/%2e/info/", "path/info/", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_PARAMETER)},
{"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_PARAMETER)},
{"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_PARAMETER)},
{"%2e/info", "info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"%u002e/info", "info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.UTF16_ENCODINGS)},

Expand Down Expand Up @@ -510,9 +510,9 @@ public static Stream<Arguments> testPathQueryTests()
{"/path/%2e/info", "/path/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"path/%2e/info/", "path/info/", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_PARAMETER)},
{"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_PARAMETER)},
{"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_PARAMETER)},
{"%2e/info", "info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
{"%2e", "", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},

Expand Down

0 comments on commit 866f451

Please sign in to comment.