Skip to content

Commit 0d0c6ed

Browse files
normanmaurervietj
normanmaurer
and
vietj
authored
Merge pull request from GHSA-5jpm-x58v-624v
* 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 <[email protected]>
1 parent d4a640d commit 0d0c6ed

File tree

4 files changed

+256
-0
lines changed

4 files changed

+256
-0
lines changed

‎codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java

+41
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
6464
*/
6565
private final HttpRequest request;
6666

67+
/**
68+
* The maximum number of fields allows by the form
69+
*/
70+
private final int maxFields;
71+
72+
/**
73+
* The maximum number of accumulated bytes when decoding a field
74+
*/
75+
private final int maxBufferedBytes;
76+
6777
/**
6878
* Default charset to use
6979
*/
@@ -175,9 +185,34 @@ public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest requ
175185
* errors
176186
*/
177187
public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset) {
188+
this(factory, request, charset, HttpPostRequestDecoder.DEFAULT_MAX_FIELDS, HttpPostRequestDecoder.DEFAULT_MAX_BUFFERED_BYTES);
189+
}
190+
191+
/**
192+
*
193+
* @param factory
194+
* the factory used to create InterfaceHttpData
195+
* @param request
196+
* the request to decode
197+
* @param charset
198+
* the charset to use as default
199+
* @param maxFields
200+
* the maximum number of fields the form can have, {@code -1} to disable
201+
* @param maxBufferedBytes
202+
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
203+
* @throws NullPointerException
204+
* for request or charset or factory
205+
* @throws ErrorDataDecoderException
206+
* if the default charset was wrong when decoding or other
207+
* errors
208+
*/
209+
public HttpPostMultipartRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset,
210+
int maxFields, int maxBufferedBytes) {
178211
this.request = checkNotNull(request, "request");
179212
this.charset = checkNotNull(charset, "charset");
180213
this.factory = checkNotNull(factory, "factory");
214+
this.maxFields = maxFields;
215+
this.maxBufferedBytes = maxBufferedBytes;
181216
// Fill default values
182217

183218
String contentTypeValue = this.request.headers().get(HttpHeaderNames.CONTENT_TYPE);
@@ -346,6 +381,9 @@ public HttpPostMultipartRequestDecoder offer(HttpContent content) {
346381
undecodedChunk.writeBytes(buf);
347382
}
348383
parseBody();
384+
if (maxBufferedBytes > 0 && undecodedChunk != null && undecodedChunk.readableBytes() > maxBufferedBytes) {
385+
throw new HttpPostRequestDecoder.TooLongFormFieldException();
386+
}
349387
if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
350388
if (undecodedChunk.refCnt() == 1) {
351389
// It's safe to call discardBytes() as we are the only owner of the buffer.
@@ -440,6 +478,9 @@ protected void addHttpData(InterfaceHttpData data) {
440478
if (data == null) {
441479
return;
442480
}
481+
if (maxFields > 0 && bodyListHttpData.size() >= maxFields) {
482+
throw new HttpPostRequestDecoder.TooManyFormFieldsException();
483+
}
443484
List<InterfaceHttpData> datas = bodyMapHttpData.get(data.getName());
444485
if (datas == null) {
445486
datas = new ArrayList<InterfaceHttpData>(1);

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

+69
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ public class HttpPostRequestDecoder implements InterfaceHttpPostRequestDecoder {
3737

3838
static final int DEFAULT_DISCARD_THRESHOLD = 10 * 1024 * 1024;
3939

40+
static final int DEFAULT_MAX_FIELDS = 128;
41+
42+
static final int DEFAULT_MAX_BUFFERED_BYTES = 1024;
43+
4044
private final InterfaceHttpPostRequestDecoder decoder;
4145

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

60+
/**
61+
*
62+
* @param request
63+
* the request to decode
64+
* @param maxFields
65+
* the maximum number of fields the form can have, {@code -1} to disable
66+
* @param maxBufferedBytes
67+
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
68+
* @throws NullPointerException
69+
* for request
70+
* @throws ErrorDataDecoderException
71+
* if the default charset was wrong when decoding or other
72+
* errors
73+
*/
74+
public HttpPostRequestDecoder(HttpRequest request, int maxFields, int maxBufferedBytes) {
75+
this(new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE), request, HttpConstants.DEFAULT_CHARSET,
76+
maxFields, maxBufferedBytes);
77+
}
78+
5679
/**
5780
*
5881
* @param factory
@@ -96,6 +119,38 @@ public HttpPostRequestDecoder(HttpDataFactory factory, HttpRequest request, Char
96119
}
97120
}
98121

122+
/**
123+
*
124+
* @param factory
125+
* the factory used to create InterfaceHttpData
126+
* @param request
127+
* the request to decode
128+
* @param charset
129+
* the charset to use as default
130+
* @param maxFields
131+
* the maximum number of fields the form can have, {@code -1} to disable
132+
* @param maxBufferedBytes
133+
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
134+
* @throws NullPointerException
135+
* for request or charset or factory
136+
* @throws ErrorDataDecoderException
137+
* if the default charset was wrong when decoding or other
138+
* errors
139+
*/
140+
public HttpPostRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset,
141+
int maxFields, int maxBufferedBytes) {
142+
ObjectUtil.checkNotNull(factory, "factory");
143+
ObjectUtil.checkNotNull(request, "request");
144+
ObjectUtil.checkNotNull(charset, "charset");
145+
146+
// Fill default values
147+
if (isMultipart(request)) {
148+
decoder = new HttpPostMultipartRequestDecoder(factory, request, charset, maxFields, maxBufferedBytes);
149+
} else {
150+
decoder = new HttpPostStandardRequestDecoder(factory, request, charset, maxFields, maxBufferedBytes);
151+
}
152+
}
153+
99154
/**
100155
* states follow NOTSTARTED PREAMBLE ( (HEADERDELIMITER DISPOSITION (FIELD |
101156
* FILEUPLOAD))* (HEADERDELIMITER DISPOSITION MIXEDPREAMBLE (MIXEDDELIMITER
@@ -338,4 +393,18 @@ public ErrorDataDecoderException(String msg, Throwable cause) {
338393
super(msg, cause);
339394
}
340395
}
396+
397+
/**
398+
* Exception when the maximum number of fields for a given form is reached
399+
*/
400+
public static final class TooManyFormFieldsException extends DecoderException {
401+
private static final long serialVersionUID = 1336267941020800769L;
402+
}
403+
404+
/**
405+
* Exception when a field content is too long
406+
*/
407+
public static final class TooLongFormFieldException extends DecoderException {
408+
private static final long serialVersionUID = 1336267941020800769L;
409+
}
341410
}

‎codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java

+44
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import io.netty.buffer.ByteBuf;
1919
import io.netty.buffer.Unpooled;
20+
import io.netty.handler.codec.DecoderException;
2021
import io.netty.handler.codec.http.HttpConstants;
2122
import io.netty.handler.codec.http.HttpContent;
2223
import io.netty.handler.codec.http.HttpRequest;
@@ -27,6 +28,8 @@
2728
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.ErrorDataDecoderException;
2829
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.MultiPartStatus;
2930
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.NotEnoughDataDecoderException;
31+
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.TooManyFormFieldsException;
32+
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.TooLongFormFieldException;
3033
import io.netty.util.ByteProcessor;
3134
import io.netty.util.internal.PlatformDependent;
3235
import io.netty.util.internal.StringUtil;
@@ -63,6 +66,16 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
6366
*/
6467
private final Charset charset;
6568

69+
/**
70+
* The maximum number of fields allows by the form
71+
*/
72+
private final int maxFields;
73+
74+
/**
75+
* The maximum number of accumulated bytes when decoding a field
76+
*/
77+
private final int maxBufferedBytes;
78+
6679
/**
6780
* Does the last chunk already received
6881
*/
@@ -148,9 +161,34 @@ public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest reque
148161
* errors
149162
*/
150163
public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset) {
164+
this(factory, request, charset, HttpPostRequestDecoder.DEFAULT_MAX_FIELDS, HttpPostRequestDecoder.DEFAULT_MAX_BUFFERED_BYTES);
165+
}
166+
167+
/**
168+
*
169+
* @param factory
170+
* the factory used to create InterfaceHttpData
171+
* @param request
172+
* the request to decode
173+
* @param charset
174+
* the charset to use as default
175+
* @param maxFields
176+
* the maximum number of fields the form can have, {@code -1} to disable
177+
* @param maxBufferedBytes
178+
* the maximum number of bytes the decoder can buffer when decoding a field, {@code -1} to disable
179+
* @throws NullPointerException
180+
* for request or charset or factory
181+
* @throws ErrorDataDecoderException
182+
* if the default charset was wrong when decoding or other
183+
* errors
184+
*/
185+
public HttpPostStandardRequestDecoder(HttpDataFactory factory, HttpRequest request, Charset charset,
186+
int maxFields, int maxBufferedBytes) {
151187
this.request = checkNotNull(request, "request");
152188
this.charset = checkNotNull(charset, "charset");
153189
this.factory = checkNotNull(factory, "factory");
190+
this.maxFields = maxFields;
191+
this.maxBufferedBytes = maxBufferedBytes;
154192
try {
155193
if (request instanceof HttpContent) {
156194
// Offer automatically if the given request is as type of HttpContent
@@ -297,6 +335,9 @@ public HttpPostStandardRequestDecoder offer(HttpContent content) {
297335
undecodedChunk.writeBytes(buf);
298336
}
299337
parseBody();
338+
if (maxBufferedBytes > 0 && undecodedChunk != null && undecodedChunk.readableBytes() > maxBufferedBytes) {
339+
throw new TooLongFormFieldException();
340+
}
300341
if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
301342
if (undecodedChunk.refCnt() == 1) {
302343
// It's safe to call discardBytes() as we are the only owner of the buffer.
@@ -387,6 +428,9 @@ protected void addHttpData(InterfaceHttpData data) {
387428
if (data == null) {
388429
return;
389430
}
431+
if (maxFields > 0 && bodyListHttpData.size() >= maxFields) {
432+
throw new TooManyFormFieldsException();
433+
}
390434
List<InterfaceHttpData> datas = bodyMapHttpData.get(data.getName());
391435
if (datas == null) {
392436
datas = new ArrayList<InterfaceHttpData>(1);

‎codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java

+102
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import io.netty.buffer.ByteBufAllocator;
2020
import io.netty.buffer.Unpooled;
2121
import io.netty.buffer.UnpooledByteBufAllocator;
22+
import io.netty.handler.codec.DecoderException;
2223
import io.netty.handler.codec.DecoderResult;
2324
import io.netty.handler.codec.http.DefaultFullHttpRequest;
2425
import io.netty.handler.codec.http.DefaultHttpContent;
@@ -1040,4 +1041,105 @@ void testHttpPostStandardRequestDecoderToDiskNameContainingUnauthorizedChar() th
10401041
}
10411042
}
10421043

1044+
@Test
1045+
public void testTooManyFormFieldsPostStandardDecoder() {
1046+
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
1047+
1048+
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, 1024, -1);
1049+
1050+
int num = 0;
1051+
while (true) {
1052+
try {
1053+
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer("foo=bar&".getBytes())));
1054+
} catch (DecoderException e) {
1055+
assertEquals(HttpPostRequestDecoder.TooManyFormFieldsException.class, e.getClass());
1056+
break;
1057+
}
1058+
assertTrue(num++ < 1024);
1059+
}
1060+
assertEquals(1024, num);
1061+
}
1062+
1063+
@Test
1064+
public void testTooManyFormFieldsPostMultipartDecoder() {
1065+
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
1066+
req.headers().add("Content-Type", "multipart/form-data;boundary=be38b42a9ad2713f");
1067+
1068+
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, 1024, -1);
1069+
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer("--be38b42a9ad2713f\n".getBytes())));
1070+
1071+
int num = 0;
1072+
while (true) {
1073+
try {
1074+
byte[] bodyBytes = ("content-disposition: form-data; name=\"title\"\n" +
1075+
"content-length: 10\n" +
1076+
"content-type: text/plain; charset=UTF-8\n" +
1077+
"\n" +
1078+
"bar-stream\n" +
1079+
"--be38b42a9ad2713f\n").getBytes();
1080+
ByteBuf content = Unpooled.wrappedBuffer(bodyBytes);
1081+
decoder.offer(new DefaultHttpContent(content));
1082+
} catch (DecoderException e) {
1083+
assertEquals(HttpPostRequestDecoder.TooManyFormFieldsException.class, e.getClass());
1084+
break;
1085+
}
1086+
assertTrue(num++ < 1024);
1087+
}
1088+
assertEquals(1024, num);
1089+
}
1090+
1091+
@Test
1092+
public void testTooLongFormFieldStandardDecoder() {
1093+
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
1094+
1095+
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, 16 * 1024);
1096+
1097+
try {
1098+
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(new byte[16 * 1024 + 1])));
1099+
fail();
1100+
} catch (DecoderException e) {
1101+
assertEquals(HttpPostRequestDecoder.TooLongFormFieldException.class, e.getClass());
1102+
}
1103+
}
1104+
1105+
@Test
1106+
public void testFieldGreaterThanMaxBufferedBytesStandardDecoder() {
1107+
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
1108+
1109+
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, 6);
1110+
1111+
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer("foo=bar".getBytes())));
1112+
}
1113+
1114+
@Test
1115+
public void testTooLongFormFieldMultipartDecoder() {
1116+
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
1117+
req.headers().add("Content-Type", "multipart/form-data;boundary=be38b42a9ad2713f");
1118+
1119+
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, 16 * 1024);
1120+
1121+
try {
1122+
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(new byte[16 * 1024 + 1])));
1123+
fail();
1124+
} catch (DecoderException e) {
1125+
assertEquals(HttpPostRequestDecoder.TooLongFormFieldException.class, e.getClass());
1126+
}
1127+
}
1128+
1129+
@Test
1130+
public void testFieldGreaterThanMaxBufferedBytesMultipartDecoder() {
1131+
HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
1132+
req.headers().add("Content-Type", "multipart/form-data;boundary=be38b42a9ad2713f");
1133+
1134+
byte[] bodyBytes = ("content-disposition: form-data; name=\"title\"\n" +
1135+
"content-length: 10\n" +
1136+
"content-type: text/plain; charset=UTF-8\n" +
1137+
"\n" +
1138+
"bar-stream\n" +
1139+
"--be38b42a9ad2713f\n").getBytes();
1140+
1141+
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req, -1, bodyBytes.length - 1);
1142+
1143+
decoder.offer(new DefaultHttpContent(Unpooled.wrappedBuffer(bodyBytes)));
1144+
}
10431145
}

0 commit comments

Comments
 (0)