Skip to content

Commit

Permalink
Merge pull request from GHSA-5jpm-x58v-624v
Browse files Browse the repository at this point in the history 
* The InterfaceHttpPostRequestDecoder form implementations does not provide hard limits for the number of fields a form can have and the number of accumulated bytes. The former can be used by sending a large amount of fields that will fill the bodyListHttpData list, the later can be used by sending a very large field filling the undecodedChunk buffer since the decoder implementation buffers the field before handling it.

This provides hard limits for both: maxFields defines the maximum number of fields the form can have, maxBufferedBytes defines the maximum number of bytes a field can cumulate. When a limit is reached, a decoder exception is thrown, letting the decoder controller take care of it.

* Set default limits for maxFields/maxUnbufferedBytes (breaking change)

* Update codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java

* Update codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java

---------

Co-authored-by: Julien Viet <julien@julienviet.com>
  • Loading branch information
@normanmaurer @vietj
normanmaurer and vietj committed Mar 21, 2024
1 parent d4a640d commit 0d0c6ed
Show file tree
Hide file tree
Showing 4 changed files with 256 additions and 0 deletions.
41 changes: 41 additions & 0 deletions .../src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
View file
Expand Up @@ -64,6 +64,16 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
*/
private final HttpRequest request;

/**
* The maximum number of fields allows by the form
*/
private final int maxFields;

/**
* The maximum number of accumulated bytes when decoding a field
*/
private final int maxBufferedBytes;

/**
* Default charset to use
*/
Expand Down Expand Up @@ -175,9 +185,34 @@ public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest requ
* errors
*/
public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset) {
this(factory, request, charset, HttpPostRequestDecoder.DEFAULT_MAX_FIELDS, HttpPostRequestDecoder.DEFAULT_MAX_BUFFERED_BYTES);
}

/**
*
* @param factory
* the factory used to create InterfaceHttpData
* @param request
* the request to decode
* @param charset
* the charset to use as default
* @param maxFields
* the maximum number of fields the form can have, {@code -1} to disable
* @param maxBufferedBytes
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
* @throws NullPointerException
* for request or charset or factory
* @throws ErrorDataDecoderException
* if the default charset was wrong when decoding or other
* errors
*/
public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset,
int maxFields, int maxBufferedBytes) {
this.request = checkNotNull(request, "request");
this.charset = checkNotNull(charset, "charset");
this.factory = checkNotNull(factory, "factory");
this.maxFields = maxFields;
this.maxBufferedBytes = maxBufferedBytes;
// Fill default values

String contentTypeValue = this.request.headers().get(HttpHeaderNames.CONTENT_TYPE);
Expand Down Expand Up @@ -346,6 +381,9 @@ public HttpPostMultipartRequestDecoder offer(HttpContent content) {
undecodedChunk.writeBytes(buf);
}
parseBody();
if (maxBufferedBytes > 0 && undecodedChunk != null && undecodedChunk.readableBytes() > maxBufferedBytes) {
throw new HttpPostRequestDecoder.TooLongFormFieldException();
}
if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
if (undecodedChunk.refCnt() == 1) {
// It's safe to call discardBytes() as we are the only owner of the buffer.
Expand Down Expand Up @@ -440,6 +478,9 @@ protected void addHttpData(InterfaceHttpData data) {
if (data == null) {
return;
}
if (maxFields > 0 && bodyListHttpData.size() >= maxFields) {
throw new HttpPostRequestDecoder.TooManyFormFieldsException();
}
List<InterfaceHttpData> datas = bodyMapHttpData.get(data.getName());
if (datas == null) {
datas = new ArrayList<InterfaceHttpData>(1);
Expand Down
69 changes: 69 additions & 0 deletions codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java
View file
Expand Up @@ -37,6 +37,10 @@ public class HttpPostRequestDecoder implements InterfaceHttpPostRequestDecoder {

static final int DEFAULT_DISCARD_THRESHOLD = 10 * 1024 * 1024;

static final int DEFAULT_MAX_FIELDS = 128;

static final int DEFAULT_MAX_BUFFERED_BYTES = 1024;

private final InterfaceHttpPostRequestDecoder decoder;

/**
Expand All @@ -53,6 +57,25 @@ public HttpPostRequestDecoder(HttpRequest request) {
this(new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE), request, HttpConstants.DEFAULT_CHARSET);
}

/**
*
* @param request
* the request to decode
* @param maxFields
* the maximum number of fields the form can have, {@code -1} to disable
* @param maxBufferedBytes
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
* @throws NullPointerException
* for request
* @throws ErrorDataDecoderException
* if the default charset was wrong when decoding or other
* errors
*/
public HttpPostRequestDecoder(HttpRequest request, int maxFields, int maxBufferedBytes) {
this(new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE), request, HttpConstants.DEFAULT_CHARSET,
maxFields, maxBufferedBytes);
}

/**
*
* @param factory
Expand Down Expand Up @@ -96,6 +119,38 @@ public HttpPostRequestDecoder(HttpDataFactory factory, HttpRequest request, Char
}
}

/**
*
* @param factory
* the factory used to create InterfaceHttpData
* @param request
* the request to decode
* @param charset
* the charset to use as default
* @param maxFields
* the maximum number of fields the form can have, {@code -1} to disable
* @param maxBufferedBytes
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
* @throws NullPointerException
* for request or charset or factory
* @throws ErrorDataDecoderException
* if the default charset was wrong when decoding or other
* errors
*/
public HttpPostRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset,
int maxFields, int maxBufferedBytes) {
ObjectUtil.checkNotNull(factory, "factory");
ObjectUtil.checkNotNull(request, "request");
ObjectUtil.checkNotNull(charset, "charset");

// Fill default values
if (isMultipart(request)) {
decoder = new HttpPostMultipartRequestDecoder(factory, request, charset, maxFields, maxBufferedBytes);
} else {
decoder = new HttpPostStandardRequestDecoder(factory, request, charset, maxFields, maxBufferedBytes);
}
}

/**
* states follow NOTSTARTED PREAMBLE ( (HEADERDELIMITER DISPOSITION (FIELD |
* FILEUPLOAD))* (HEADERDELIMITER DISPOSITION MIXEDPREAMBLE (MIXEDDELIMITER
Expand Down Expand Up @@ -338,4 +393,18 @@ public ErrorDataDecoderException(String msg, Throwable cause) {
super(msg, cause);
}
}

/**
* Exception when the maximum number of fields for a given form is reached
*/
public static final class TooManyFormFieldsException extends DecoderException {
private static final long serialVersionUID = 1336267941020800769L;
}

/**
* Exception when a field content is too long
*/
public static final class TooLongFormFieldException extends DecoderException {
private static final long serialVersionUID = 1336267941020800769L;
}
}
44 changes: 44 additions & 0 deletions ...p/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
View file
Expand Up @@ -17,6 +17,7 @@

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.http.HttpConstants;
import io.netty.handler.codec.http.HttpContent;
import io.netty.handler.codec.http.HttpRequest;
Expand All @@ -27,6 +28,8 @@
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.ErrorDataDecoderException;
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.MultiPartStatus;
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.NotEnoughDataDecoderException;
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.TooManyFormFieldsException;
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.TooLongFormFieldException;
import io.netty.util.ByteProcessor;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.StringUtil;
Expand Down Expand Up @@ -63,6 +66,16 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
*/
private final Charset charset;

/**
* The maximum number of fields allows by the form
*/
private final int maxFields;

/**
* The maximum number of accumulated bytes when decoding a field
*/
private final int maxBufferedBytes;

/**
* Does the last chunk already received
*/
Expand Down Expand Up @@ -148,9 +161,34 @@ public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest reque
* errors
*/
public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset) {
this(factory, request, charset, HttpPostRequestDecoder.DEFAULT_MAX_FIELDS, HttpPostRequestDecoder.DEFAULT_MAX_BUFFERED_BYTES);
}

/**
*
* @param factory
* the factory used to create InterfaceHttpData
* @param request
* the request to decode
* @param charset
* the charset to use as default
* @param maxFields
* the maximum number of fields the form can have, {@code -1} to disable
* @param maxBufferedBytes
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
* @throws NullPointerException
* for request or charset or factory
* @throws ErrorDataDecoderException
* if the default charset was wrong when decoding or other
* errors
*/
public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset,
int maxFields, int maxBufferedBytes) {
this.request = checkNotNull(request, "request");
this.charset = checkNotNull(charset, "charset");
this.factory = checkNotNull(factory, "factory");
this.maxFields = maxFields;
this.maxBufferedBytes = maxBufferedBytes;
try {
if (request instanceof HttpContent) {
// Offer automatically if the given request is as type of HttpContent
Expand Down Expand Up @@ -297,6 +335,9 @@ public HttpPostStandardRequestDecoder offer(HttpContent content) {
undecodedChunk.writeBytes(buf);
}
parseBody();
if (maxBufferedBytes > 0 && undecodedChunk != null && undecodedChunk.readableBytes() > maxBufferedBytes) {
throw new TooLongFormFieldException();
}
if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
if (undecodedChunk.refCnt() == 1) {
// It's safe to call discardBytes() as we are the only owner of the buffer.
Expand Down Expand Up @@ -387,6 +428,9 @@ protected void addHttpData(InterfaceHttpData data) {
if (data == null) {
return;
}
if (maxFields > 0 && bodyListHttpData.size() >= maxFields) {
throw new TooManyFormFieldsException();
}
List<InterfaceHttpData> datas = bodyMapHttpData.get(data.getName());
if (datas == null) {
datas = new ArrayList<InterfaceHttpData>(1);
Expand Down
102 changes: 102 additions & 0 deletions ...-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java
View file
Expand Up @@ -19,6 +19,7 @@
import io.netty.buffer.ByteBufAllocator;
import io.netty.buffer.Unpooled;
import io.netty.buffer.UnpooledByteBufAllocator;
import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.DecoderResult;
import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultHttpContent;
Expand Down Expand Up @@ -1040,4 +1041,105 @@ void testHttpPostStandardRequestDecoderToDiskNameContainingUnauthorizedChar() th
}
}

@Test
public void testTooManyFormFieldsPostStandardDecoder() {
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");

HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, 1024, -1);

int num = 0;
while (true) {
try {
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer("foo=bar&".getBytes())));
} catch (DecoderException e) {
assertEquals(HttpPostRequestDecoder.TooManyFormFieldsException.class, e.getClass());
break;
}
assertTrue(num++ < 1024);
}
assertEquals(1024, num);
}

@Test
public void testTooManyFormFieldsPostMultipartDecoder() {
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
req.headers().add("Content-Type", "multipart/form-data;boundary=be38b42a9ad2713f");

HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, 1024, -1);
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer("--be38b42a9ad2713f\n".getBytes())));

int num = 0;
while (true) {
try {
byte[] bodyBytes = ("content-disposition: form-data; name=\"title\"\n" +
"content-length: 10\n" +
"content-type: text/plain; charset=UTF-8\n" +
"\n" +
"bar-stream\n" +
"--be38b42a9ad2713f\n").getBytes();
ByteBuf content = Unpooled.wrappedBuffer(bodyBytes);
decoder.offer(new DefaultHttpContent(content));
} catch (DecoderException e) {
assertEquals(HttpPostRequestDecoder.TooManyFormFieldsException.class, e.getClass());
break;
}
assertTrue(num++ < 1024);
}
assertEquals(1024, num);
}

@Test
public void testTooLongFormFieldStandardDecoder() {
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");

HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, 16 * 1024);

try {
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(new byte[16 * 1024 + 1])));
fail();
} catch (DecoderException e) {
assertEquals(HttpPostRequestDecoder.TooLongFormFieldException.class, e.getClass());
}
}

@Test
public void testFieldGreaterThanMaxBufferedBytesStandardDecoder() {
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");

HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, 6);

decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer("foo=bar".getBytes())));
}

@Test
public void testTooLongFormFieldMultipartDecoder() {
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
req.headers().add("Content-Type", "multipart/form-data;boundary=be38b42a9ad2713f");

HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, 16 * 1024);

try {
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(new byte[16 * 1024 + 1])));
fail();
} catch (DecoderException e) {
assertEquals(HttpPostRequestDecoder.TooLongFormFieldException.class, e.getClass());
}
}

@Test
public void testFieldGreaterThanMaxBufferedBytesMultipartDecoder() {
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
req.headers().add("Content-Type", "multipart/form-data;boundary=be38b42a9ad2713f");

byte[] bodyBytes = ("content-disposition: form-data; name=\"title\"\n" +
"content-length: 10\n" +
"content-type: text/plain; charset=UTF-8\n" +
"\n" +
"bar-stream\n" +
"--be38b42a9ad2713f\n").getBytes();

HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, bodyBytes.length - 1);

decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(bodyBytes)));
}
}

0 comments on commit 0d0c6ed

Please sign in to comment.