Skip to content

Commit

Permalink
Fix CVE-2024-6763 (#12532)
Browse files Browse the repository at this point in the history 
Back port the fix for [CVE-2024-6763](GHSA-qh8g-58pp-2wxh).

Better validation of authority in HttpURI.  No known exploits.
  • Loading branch information
@gregw
gregw authored Nov 14, 2024
1 parent ab013e6 commit db8bb7a
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 12 deletions.
5 changes: 5 additions & 0 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ static EnumSet<HttpComplianceSection> 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;
Expand Down Expand Up @@ -217,6 +218,7 @@ public EnumSet<HttpComplianceSection> sections()
}

private static final EnumMap<HttpURI.Violation, HttpComplianceSection> __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
Expand All @@ -242,6 +244,9 @@ public EnumSet<HttpComplianceSection> 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();
}
Expand Down
1 change: 1 addition & 0 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
127 changes: 120 additions & 7 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<pchar>
//
// 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;
Expand Down Expand Up @@ -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<x>
int encodedValue = 0; // the partial encoded value
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -614,7 +727,7 @@ else if (c == '/')
dot |= segment == i;
break;
case '%':
encodedPath = true;
encoded = true;
encodedUtf16 = false;
encodedCharacters = 2;
encodedValue = 0;
Expand Down Expand Up @@ -642,7 +755,7 @@ else if (c == '/')
state = State.FRAGMENT;
break;
case '/':
encodedPath = true;
encoded = true;
segment = i + 1;
state = State.PATH;
break;
Expand Down Expand Up @@ -721,7 +834,7 @@ else if (c == '/')
throw new IllegalStateException(state.toString());
}

if (!encodedPath && !dot)
if (!encoded && !dot)
{
if (_param == null)
_decodedPath = _path;
Expand Down
37 changes: 32 additions & 5 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -875,4 +870,36 @@ public void testRelativePathWithAuthority()
uri.setPath("");
assertEquals("//host", uri.toString());
}

public static Stream<String> 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));
}
}

0 comments on commit db8bb7a

Please sign in to comment.