diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java index 14f60045d3c8..521e029fcb85 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java @@ -135,6 +135,7 @@ static EnumSet sectionsBySpec(String spec) HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS, HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS, HttpComplianceSection.NO_UTF16_ENCODINGS, + HttpComplianceSection.NO_USER_INFO, HttpComplianceSection.NO_AMBIGUOUS_EMPTY_SEGMENT, HttpComplianceSection.NO_AMBIGUOUS_PATH_ENCODING)); break; @@ -217,6 +218,7 @@ public EnumSet sections() } private static final EnumMap __uriViolations = new EnumMap<>(HttpURI.Violation.class); + static { // create a map from Violation to compliance in a loop, so that any new violations added are detected with ISE @@ -242,6 +244,9 @@ public EnumSet sections() case UTF16: __uriViolations.put(violation, HttpComplianceSection.NO_UTF16_ENCODINGS); break; + case USER_INFO: + __uriViolations.put(violation, HttpComplianceSection.NO_USER_INFO); + break; default: throw new IllegalStateException(); } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java index 5385d5a57d7d..d54069754e80 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java @@ -36,6 +36,7 @@ public enum HttpComplianceSection NO_AMBIGUOUS_PATH_SEPARATORS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path separators"), NO_AMBIGUOUS_PATH_PARAMETERS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path parameters"), NO_UTF16_ENCODINGS("https://www.w3.org/International/iri-edit/draft-duerst-iri.html#anchor29", "UTF16 encoding"), + NO_USER_INFO("https://datatracker.ietf.org/doc/html/rfc9110#name-deprecation-of-userinfo-in-", "User info in authority"), NO_AMBIGUOUS_EMPTY_SEGMENT("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI empty segment"), NO_AMBIGUOUS_PATH_ENCODING("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path encoding"); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index 8ecc4aefded8..a356e8035bde 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -122,7 +122,11 @@ enum Violation /** * Non standard UTF-16 encoding eg {@code /foo%u2192bar}. */ - UTF16("Non standard UTF-16 encoding"); + UTF16("Non standard UTF-16 encoding"), + /** + * User info in authority + */ + USER_INFO("User info in Authority"); private final String _message; @@ -155,6 +159,84 @@ String getMessage() __ambiguousSegments.put("%u002e%u002e", Boolean.TRUE); } + private static final boolean[] __unreservedPctEncodedSubDelims; + + private static boolean isDigit(char c) + { + return (c >= '0') && (c <= '9'); + } + + private static boolean isHexDigit(char c) + { + return (((c >= 'a') && (c <= 'f')) || // ALPHA (lower) + ((c >= 'A') && (c <= 'F')) || // ALPHA (upper) + ((c >= '0') && (c <= '9'))); + } + + private static boolean isUnreserved(char c) + { + return (((c >= 'a') && (c <= 'z')) || // ALPHA (lower) + ((c >= 'A') && (c <= 'Z')) || // ALPHA (upper) + ((c >= '0') && (c <= '9')) || // DIGIT + (c == '-') || (c == '.') || (c == '_') || (c == '~')); + } + + private static boolean isSubDelim(char c) + { + return c == '!' || c == '$' || c == '&' || c == '\'' || c == '(' || c == ')' || c == '*' || c == '+' || c == ',' || c == ';' || c == '='; + } + + static boolean isUnreservedPctEncodedOrSubDelim(char c) + { + return c < __unreservedPctEncodedSubDelims.length && __unreservedPctEncodedSubDelims[c]; + } + + static + { + // Establish allowed and disallowed characters per the path rules of + // https://datatracker.ietf.org/doc/html/rfc3986#section-3.3 + // ABNF + // path = path-abempty ; begins with "/" or is empty + // / path-absolute ; begins with "/" but not "//" + // / path-noscheme ; begins with a non-colon segment + // / path-rootless ; begins with a segment + // / path-empty ; zero characters + // path-abempty = *( "/" segment ) + // path-absolute = "/" [ segment-nz *( "/" segment ) ] + // path-noscheme = segment-nz-nc *( "/" segment ) + // path-rootless = segment-nz *( "/" segment ) + // path-empty = 0 + // + // segment = *pchar + // segment-nz = 1*pchar + // segment-nz-nc = 1*( unreserved / pct-encoded / sub-delims / "@" ) + // ; non-zero-length segment without any colon ":" + // pchar = unreserved / pct-encoded / sub-delims / ":" / "@" + // pct-encoded = "%" HEXDIG HEXDIG + // + // unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" + // reserved = gen-delims / sub-delims + // gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@" + // sub-delims = "!" / "$" / "&" / "'" / "(" / ")" + // / "*" / "+" / "," / ";" / "=" + // + // authority = [ userinfo "@" ] host [ ":" port ] + // userinfo = *( unreserved / pct-encoded / sub-delims / ":" ) + // host = IP-literal / IPv4address / reg-name + // port = *DIGIT + // + // reg-name = *( unreserved / pct-encoded / sub-delims ) + // + // we are limited to US-ASCII per https://datatracker.ietf.org/doc/html/rfc3986#section-2 + __unreservedPctEncodedSubDelims = new boolean[128]; + + for (int i = 0; i < __unreservedPctEncodedSubDelims.length; i++) + { + char c = (char)i; + __unreservedPctEncodedSubDelims[i] = isUnreserved(c) || c == '%' || isSubDelim(c); + } + } + private String _scheme; private String _user; private String _host; @@ -334,7 +416,7 @@ private void parse(State state, final String uri, final int offset, final int en int mark = offset; // the start of the current section being parsed int pathMark = 0; // the start of the path section int segment = 0; // the start of the current segment within the path - boolean encodedPath = false; // set to true if the path contains % encoded characters + boolean encoded = false; // set to true if the path contains % encoded characters boolean encodedUtf16 = false; // Is the current encoding for UTF16? int encodedCharacters = 0; // partial state of parsing a % encoded character int encodedValue = 0; // the partial encoded value @@ -378,7 +460,7 @@ private void parse(State state, final String uri, final int offset, final int en state = State.ASTERISK; break; case '%': - encodedPath = true; + encoded = true; encodedCharacters = 2; encodedValue = 0; mark = pathMark = segment = i; @@ -431,7 +513,7 @@ private void parse(State state, final String uri, final int offset, final int en break; case '%': // must have been in an encoded path - encodedPath = true; + encoded = true; encodedCharacters = 2; encodedValue = 0; state = State.PATH; @@ -481,7 +563,10 @@ private void parse(State state, final String uri, final int offset, final int en switch (c) { case '/': + if (encodedCharacters > 0) + throw new IllegalArgumentException("Bad authority"); _host = uri.substring(mark, i); + encoded = false; pathMark = mark = i; segment = mark + 1; state = State.PATH; @@ -496,12 +581,35 @@ private void parse(State state, final String uri, final int offset, final int en if (_user != null) throw new IllegalArgumentException("Bad authority"); _user = uri.substring(mark, i); + _violations.add(Violation.USER_INFO); mark = i + 1; break; case '[': + if (i != mark) + throw new IllegalArgumentException("Bad authority"); state = State.IPV6; break; + case '%': + if (encodedCharacters > 0) + throw new IllegalArgumentException("Bad authority"); + encodedCharacters = 2; + encoded = true; + break; + case '#': + case ';': + throw new IllegalArgumentException("Bad authority"); + default: + if (encodedCharacters > 0) + { + if (!isHexDigit(c)) + throw new IllegalArgumentException("Bad authority"); + encodedCharacters--; + } + else if (!isUnreservedPctEncodedOrSubDelim(c)) + { + throw new IllegalArgumentException("Bad authority"); + } break; } break; @@ -526,7 +634,11 @@ private void parse(State state, final String uri, final int offset, final int en state = State.PATH; } break; + case ':': + break; default: + if (!isHexDigit(c)) + throw new IllegalArgumentException("Bad authority"); break; } break; @@ -539,6 +651,7 @@ private void parse(State state, final String uri, final int offset, final int en throw new IllegalArgumentException("Bad authority"); // It wasn't a port, but a password! _user = _host + ":" + uri.substring(mark, i); + _violations.add(Violation.USER_INFO); mark = i + 1; state = State.HOST; } @@ -614,7 +727,7 @@ else if (c == '/') dot |= segment == i; break; case '%': - encodedPath = true; + encoded = true; encodedUtf16 = false; encodedCharacters = 2; encodedValue = 0; @@ -642,7 +755,7 @@ else if (c == '/') state = State.FRAGMENT; break; case '/': - encodedPath = true; + encoded = true; segment = i + 1; state = State.PATH; break; @@ -721,7 +834,7 @@ else if (c == '/') throw new IllegalStateException(state.toString()); } - if (!encodedPath && !dot) + if (!encoded && !dot) { if (_param == null) _decodedPath = _path; diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java index 6f31cbc95edd..407bfc3e4880 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java @@ -87,11 +87,6 @@ public void testParse() uri.parse("http://foo/bar"); assertThat(uri.getHost(), is("foo")); assertThat(uri.getPath(), is("/bar")); - - // We do allow nulls if not encoded. This can be used for testing 2nd line of defence. - uri.parse("http://fo\000/bar"); - assertThat(uri.getHost(), is("fo\000")); - assertThat(uri.getPath(), is("/bar")); } @Test @@ -875,4 +870,36 @@ public void testRelativePathWithAuthority() uri.setPath(""); assertEquals("//host", uri.toString()); } + + public static Stream badAuthorities() + { + return Stream.of( + "http://#host/path", + "https:// host/path", + "https://h st/path", + "https://h\000st/path", + "https://h%GGst/path", + "https://host%/path", + "https://host%0/path", + "https://host%u001f/path", + "https://host%:8080/path", + "https://host%0:8080/path", + "https://user%@host/path", + "https://user%0@host/path", + "https://host:notport/path", + "https://user@host:notport/path", + "https://user:password@host:notport/path", + "https://user @host.com/", + "https://user#@host.com/", + "https://[notIpv6]/", + "https://bad[0::1::2::3::4]/" + ); + } + + @ParameterizedTest + @MethodSource("badAuthorities") + public void testBadAuthority(String uri) + { + assertThrows(IllegalArgumentException.class, () -> new HttpURI(uri)); + } }