257 lines
11 KiB
Diff
257 lines
11 KiB
Diff
|
|
From: Markus Koschany <apo@debian.org>
|
||
|
|
Date: Tue, 26 Sep 2023 21:06:42 +0200
|
||
|
|
Subject: CVE-2023-40167
|
||
|
|
|
||
|
|
Origin: https://github.com/eclipse/jetty.project/commit/e4d596eafc887bcd813ae6e28295b5ce327def47
|
||
|
|
---
|
||
|
|
.../java/org/eclipse/jetty/http/HttpParser.java | 47 +++++++-------
|
||
|
|
.../org/eclipse/jetty/http/HttpParserTest.java | 71 +++++-----------------
|
||
|
|
2 files changed, 38 insertions(+), 80 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
|
||
|
|
index 2abc4b6..c045498 100644
|
||
|
|
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
|
||
|
|
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
|
||
|
|
@@ -501,7 +501,7 @@ public class HttpParser
|
||
|
|
/* Quick lookahead for the start state looking for a request method or a HTTP version,
|
||
|
|
* otherwise skip white space until something else to parse.
|
||
|
|
*/
|
||
|
|
- private boolean quickStart(ByteBuffer buffer)
|
||
|
|
+ private void quickStart(ByteBuffer buffer)
|
||
|
|
{
|
||
|
|
if (_requestHandler!=null)
|
||
|
|
{
|
||
|
|
@@ -512,7 +512,7 @@ public class HttpParser
|
||
|
|
buffer.position(buffer.position()+_methodString.length()+1);
|
||
|
|
|
||
|
|
setState(State.SPACE1);
|
||
|
|
- return false;
|
||
|
|
+ return;
|
||
|
|
}
|
||
|
|
}
|
||
|
|
else if (_responseHandler!=null)
|
||
|
|
@@ -522,7 +522,7 @@ public class HttpParser
|
||
|
|
{
|
||
|
|
buffer.position(buffer.position()+_version.asString().length()+1);
|
||
|
|
setState(State.SPACE1);
|
||
|
|
- return false;
|
||
|
|
+ return;
|
||
|
|
}
|
||
|
|
}
|
||
|
|
|
||
|
|
@@ -543,7 +543,7 @@ public class HttpParser
|
||
|
|
_string.setLength(0);
|
||
|
|
_string.append(t.getChar());
|
||
|
|
setState(_requestHandler!=null?State.METHOD:State.RESPONSE_VERSION);
|
||
|
|
- return false;
|
||
|
|
+ return;
|
||
|
|
}
|
||
|
|
case OTEXT:
|
||
|
|
case SPACE:
|
||
|
|
@@ -561,7 +561,6 @@ public class HttpParser
|
||
|
|
throw new BadMessageException(HttpStatus.BAD_REQUEST_400);
|
||
|
|
}
|
||
|
|
}
|
||
|
|
- return false;
|
||
|
|
}
|
||
|
|
|
||
|
|
/* ------------------------------------------------------------------------------- */
|
||
|
|
@@ -979,12 +978,13 @@ public class HttpParser
|
||
|
|
switch (_header)
|
||
|
|
{
|
||
|
|
case CONTENT_LENGTH:
|
||
|
|
+ long contentLength = convertContentLength(_valueString);
|
||
|
|
if (_hasContentLength)
|
||
|
|
{
|
||
|
|
if(complianceViolation(MULTIPLE_CONTENT_LENGTHS))
|
||
|
|
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,MULTIPLE_CONTENT_LENGTHS.description);
|
||
|
|
- if (convertContentLength(_valueString)!=_contentLength)
|
||
|
|
- throw new BadMessageException(HttpStatus.BAD_REQUEST_400,MULTIPLE_CONTENT_LENGTHS.description);
|
||
|
|
+ if (contentLength != _contentLength)
|
||
|
|
+ throw new BadMessageException(HttpStatus.BAD_REQUEST_400, MULTIPLE_CONTENT_LENGTHS.getDescription());
|
||
|
|
}
|
||
|
|
_hasContentLength = true;
|
||
|
|
|
||
|
|
@@ -993,11 +993,8 @@ public class HttpParser
|
||
|
|
|
||
|
|
if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
|
||
|
|
{
|
||
|
|
- _contentLength=convertContentLength(_valueString);
|
||
|
|
- if (_contentLength <= 0)
|
||
|
|
- _endOfContent=EndOfContent.NO_CONTENT;
|
||
|
|
- else
|
||
|
|
- _endOfContent=EndOfContent.CONTENT_LENGTH;
|
||
|
|
+ _contentLength = contentLength;
|
||
|
|
+ _endOfContent = EndOfContent.CONTENT_LENGTH;
|
||
|
|
}
|
||
|
|
break;
|
||
|
|
|
||
|
|
@@ -1085,15 +1082,21 @@ public class HttpParser
|
||
|
|
|
||
|
|
private long convertContentLength(String valueString)
|
||
|
|
{
|
||
|
|
- try
|
||
|
|
- {
|
||
|
|
- return Long.parseLong(valueString);
|
||
|
|
- }
|
||
|
|
- catch(NumberFormatException e)
|
||
|
|
+ if (valueString == null || valueString.length() == 0)
|
||
|
|
+ throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());
|
||
|
|
+
|
||
|
|
+ long value = 0;
|
||
|
|
+ int length = valueString.length();
|
||
|
|
+
|
||
|
|
+ for (int i = 0; i < length; i++)
|
||
|
|
{
|
||
|
|
- LOG.ignore(e);
|
||
|
|
- throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Invalid Content-Length Value",e);
|
||
|
|
+ char c = valueString.charAt(i);
|
||
|
|
+ if (c < '0' || c > '9')
|
||
|
|
+ throw new BadMessageException("Invalid Content-Length Value", new NumberFormatException());
|
||
|
|
+
|
||
|
|
+ value = Math.addExact(Math.multiplyExact(value, 10L), c - '0');
|
||
|
|
}
|
||
|
|
+ return value;
|
||
|
|
}
|
||
|
|
|
||
|
|
/* ------------------------------------------------------------------------------- */
|
||
|
|
@@ -1485,12 +1488,11 @@ public class HttpParser
|
||
|
|
_methodString=null;
|
||
|
|
_endOfContent=EndOfContent.UNKNOWN_CONTENT;
|
||
|
|
_header=null;
|
||
|
|
- if (quickStart(buffer))
|
||
|
|
- return true;
|
||
|
|
+ quickStart(buffer);
|
||
|
|
}
|
||
|
|
|
||
|
|
// Request/response line
|
||
|
|
- if (_state.ordinal()>= State.START.ordinal() && _state.ordinal()<State.HEADER.ordinal())
|
||
|
|
+ if (_state.ordinal() < State.HEADER.ordinal())
|
||
|
|
{
|
||
|
|
if (parseLine(buffer))
|
||
|
|
return true;
|
||
|
|
@@ -1994,7 +1996,6 @@ public class HttpParser
|
||
|
|
}
|
||
|
|
|
||
|
|
/* ------------------------------------------------------------------------------- */
|
||
|
|
- @SuppressWarnings("serial")
|
||
|
|
private static class IllegalCharacterException extends BadMessageException
|
||
|
|
{
|
||
|
|
private IllegalCharacterException(State state,HttpTokens.Token token,ByteBuffer buffer)
|
||
|
|
diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
|
||
|
|
index dc340c1..c1d59cd 100644
|
||
|
|
--- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
|
||
|
|
+++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
|
||
|
|
@@ -23,6 +23,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
|
||
|
|
import static org.hamcrest.Matchers.contains;
|
||
|
|
import static org.hamcrest.Matchers.containsString;
|
||
|
|
import static org.hamcrest.Matchers.is;
|
||
|
|
+import static org.hamcrest.Matchers.notNullValue;
|
||
|
|
import static org.hamcrest.Matchers.nullValue;
|
||
|
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||
|
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||
|
|
@@ -1653,7 +1654,7 @@ public class HttpParserTest
|
||
|
|
}
|
||
|
|
|
||
|
|
@Test
|
||
|
|
- public void testUnknownReponseVersion() throws Exception
|
||
|
|
+ public void testUnknownResponseVersion()
|
||
|
|
{
|
||
|
|
ByteBuffer buffer = BufferUtil.toBuffer(
|
||
|
|
"HPPT/7.7 200 OK\r\n"
|
||
|
|
@@ -1797,65 +1798,21 @@ public class HttpParserTest
|
||
|
|
assertEquals(HttpParser.State.CLOSED, parser.getState());
|
||
|
|
}
|
||
|
|
|
||
|
|
- @Test
|
||
|
|
- public void testBadContentLength0() throws Exception
|
||
|
|
- {
|
||
|
|
- ByteBuffer buffer = BufferUtil.toBuffer(
|
||
|
|
- "GET / HTTP/1.0\r\n"
|
||
|
|
- + "Content-Length: abc\r\n"
|
||
|
|
- + "Connection: close\r\n"
|
||
|
|
- + "\r\n");
|
||
|
|
-
|
||
|
|
- HttpParser.RequestHandler handler = new Handler();
|
||
|
|
- HttpParser parser = new HttpParser(handler);
|
||
|
|
-
|
||
|
|
- parser.parseNext(buffer);
|
||
|
|
- assertEquals("GET", _methodOrVersion);
|
||
|
|
- assertEquals("Invalid Content-Length Value", _bad);
|
||
|
|
- assertFalse(buffer.hasRemaining());
|
||
|
|
- assertEquals(HttpParser.State.CLOSE, parser.getState());
|
||
|
|
- parser.atEOF();
|
||
|
|
- parser.parseNext(BufferUtil.EMPTY_BUFFER);
|
||
|
|
- assertEquals(HttpParser.State.CLOSED, parser.getState());
|
||
|
|
- }
|
||
|
|
-
|
||
|
|
- @Test
|
||
|
|
- public void testBadContentLength1() throws Exception
|
||
|
|
- {
|
||
|
|
- ByteBuffer buffer = BufferUtil.toBuffer(
|
||
|
|
- "GET / HTTP/1.0\r\n"
|
||
|
|
- + "Content-Length: 9999999999999999999999999999999999999999999999\r\n"
|
||
|
|
- + "Connection: close\r\n"
|
||
|
|
- + "\r\n");
|
||
|
|
-
|
||
|
|
- HttpParser.RequestHandler handler = new Handler();
|
||
|
|
- HttpParser parser = new HttpParser(handler);
|
||
|
|
-
|
||
|
|
- parser.parseNext(buffer);
|
||
|
|
- assertEquals("GET", _methodOrVersion);
|
||
|
|
- assertEquals("Invalid Content-Length Value", _bad);
|
||
|
|
- assertFalse(buffer.hasRemaining());
|
||
|
|
- assertEquals(HttpParser.State.CLOSE, parser.getState());
|
||
|
|
- parser.atEOF();
|
||
|
|
- parser.parseNext(BufferUtil.EMPTY_BUFFER);
|
||
|
|
- assertEquals(HttpParser.State.CLOSED, parser.getState());
|
||
|
|
- }
|
||
|
|
|
||
|
|
- @Test
|
||
|
|
- public void testBadContentLength2() throws Exception
|
||
|
|
+ public void testBadContentLengths(String contentLength)
|
||
|
|
{
|
||
|
|
ByteBuffer buffer = BufferUtil.toBuffer(
|
||
|
|
- "GET / HTTP/1.0\r\n"
|
||
|
|
- + "Content-Length: 1.5\r\n"
|
||
|
|
- + "Connection: close\r\n"
|
||
|
|
- + "\r\n");
|
||
|
|
+ "GET /test HTTP/1.1\r\n" +
|
||
|
|
+ "Host: localhost\r\n" +
|
||
|
|
+ "Content-Length: " + contentLength + "\r\n" +
|
||
|
|
+ "\r\n" +
|
||
|
|
+ "1234567890\r\n");
|
||
|
|
|
||
|
|
HttpParser.RequestHandler handler = new Handler();
|
||
|
|
- HttpParser parser = new HttpParser(handler);
|
||
|
|
+ HttpParser parser = new HttpParser(handler, HttpCompliance.RFC2616_LEGACY);
|
||
|
|
+ parseAll(parser, buffer);
|
||
|
|
|
||
|
|
- parser.parseNext(buffer);
|
||
|
|
- assertEquals("GET", _methodOrVersion);
|
||
|
|
- assertEquals("Invalid Content-Length Value", _bad);
|
||
|
|
+ assertThat(_bad, notNullValue());
|
||
|
|
assertFalse(buffer.hasRemaining());
|
||
|
|
assertEquals(HttpParser.State.CLOSE, parser.getState());
|
||
|
|
parser.atEOF();
|
||
|
|
@@ -2066,7 +2023,7 @@ public class HttpParserTest
|
||
|
|
@Test
|
||
|
|
public void testBadIPv6Host() throws Exception
|
||
|
|
{
|
||
|
|
- try(StacklessLogging s = new StacklessLogging(HttpParser.class))
|
||
|
|
+ try (StacklessLogging ignored = new StacklessLogging(HttpParser.class))
|
||
|
|
{
|
||
|
|
ByteBuffer buffer = BufferUtil.toBuffer(
|
||
|
|
"GET / HTTP/1.1\r\n"
|
||
|
|
@@ -2254,8 +2211,8 @@ public class HttpParserTest
|
||
|
|
private String _methodOrVersion;
|
||
|
|
private String _uriOrStatus;
|
||
|
|
private String _versionOrReason;
|
||
|
|
- private List<HttpField> _fields = new ArrayList<>();
|
||
|
|
- private List<HttpField> _trailers = new ArrayList<>();
|
||
|
|
+ private final List<HttpField> _fields = new ArrayList<>();
|
||
|
|
+ private final List<HttpField> _trailers = new ArrayList<>();
|
||
|
|
private String[] _hdr;
|
||
|
|
private String[] _val;
|
||
|
|
private int _headers;
|