Skip to content

Commit

Permalink
Add config options for form parsing limits (#10967)
Browse files Browse the repository at this point in the history
For CVE-2024-29025, netty introduced form parsing limits with restrictive defaults. This PR adds config options for them.

Fixes #10825
  • Loading branch information
yawkat authored Jul 10, 2024
1 parent ba0e749 commit 8f0a7ab
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public final class FormDataHttpContentProcessor {
protected final long advertisedLength;
protected final long requestMaxSize;
protected final AtomicLong receivedLength = new AtomicLong();
protected final HttpServerConfiguration configuration;
protected final NettyHttpServerConfiguration configuration;
private final InterfaceHttpPostRequestDecoder decoder;
private final long partMaxSize;

Expand All @@ -79,7 +79,7 @@ public final class FormDataHttpContentProcessor {
* @param nettyHttpRequest The {@link NettyHttpRequest}
* @param configuration The {@link NettyHttpServerConfiguration}
*/
public FormDataHttpContentProcessor(NettyHttpRequest<?> nettyHttpRequest, HttpServerConfiguration configuration) {
public FormDataHttpContentProcessor(NettyHttpRequest<?> nettyHttpRequest, NettyHttpServerConfiguration configuration) {
this.nettyHttpRequest = nettyHttpRequest;
this.advertisedLength = nettyHttpRequest.getContentLength();
this.requestMaxSize = configuration.getMaxRequestSize();
Expand All @@ -90,9 +90,9 @@ public FormDataHttpContentProcessor(NettyHttpRequest<?> nettyHttpRequest, HttpSe
// prevent the decoders from immediately parsing the content
HttpRequest nativeRequest = nettyHttpRequest.toHttpRequestWithoutBody();
if (HttpPostRequestDecoder.isMultipart(nativeRequest)) {
this.decoder = new HttpPostMultipartRequestDecoder(factory, nativeRequest, characterEncoding);
this.decoder = new HttpPostMultipartRequestDecoder(factory, nativeRequest, characterEncoding, configuration.getFormMaxFields(), configuration.getFormMaxBufferedBytes());
} else {
this.decoder = new HttpPostStandardRequestDecoder(factory, nativeRequest, characterEncoding);
this.decoder = new HttpPostStandardRequestDecoder(factory, nativeRequest, characterEncoding, configuration.getFormMaxFields(), configuration.getFormMaxBufferedBytes());
}
this.partMaxSize = multipart.getMaxFileSize();
}
Expand Down Expand Up @@ -157,6 +157,10 @@ protected void onData(ByteBufHolder message, Collection<? super InterfaceHttpDat
} else {
throw e;
}
} catch (HttpPostRequestDecoder.TooManyFormFieldsException e) {
throw new ContentLengthExceededException("Number of form fields exceeds configured limit of [" + configuration.getFormMaxFields() + "]");
} catch (HttpPostRequestDecoder.TooLongFormFieldException e) {
throw new ContentLengthExceededException("Length of buffered form field exceeds configured limit of [" + configuration.getFormMaxBufferedBytes() + "]");
} finally {
httpContent.release();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@
import io.micronaut.http.body.MessageBodyHandlerRegistry;
import io.micronaut.http.body.MessageBodyReader;
import io.micronaut.http.codec.CodecException;
import io.micronaut.http.server.HttpServerConfiguration;
import io.micronaut.http.server.netty.FormDataHttpContentProcessor;
import io.micronaut.http.server.netty.NettyHttpRequest;
import io.micronaut.http.server.netty.body.AvailableNettyByteBody;
import io.micronaut.http.server.netty.body.ImmediateMultiObjectBody;
import io.micronaut.http.server.netty.body.ImmediateSingleObjectBody;
import io.micronaut.http.server.netty.configuration.NettyHttpServerConfiguration;
import io.micronaut.web.router.RouteInfo;
import io.netty.buffer.ByteBuf;
import io.netty.handler.codec.http.DefaultLastHttpContent;
Expand All @@ -56,11 +56,11 @@

final class NettyBodyAnnotationBinder<T> extends DefaultBodyAnnotationBinder<T> {
private static final Set<Class<?>> RAW_BODY_TYPES = CollectionUtils.setOf(String.class, byte[].class, ByteBuffer.class, InputStream.class);
final HttpServerConfiguration httpServerConfiguration;
final NettyHttpServerConfiguration httpServerConfiguration;
final MessageBodyHandlerRegistry bodyHandlerRegistry;

NettyBodyAnnotationBinder(ConversionService conversionService,
HttpServerConfiguration httpServerConfiguration,
NettyHttpServerConfiguration httpServerConfiguration,
MessageBodyHandlerRegistry bodyHandlerRegistry) {
super(conversionService);
this.httpServerConfiguration = httpServerConfiguration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package io.micronaut.http.server.netty.binders;

import io.micronaut.context.BeanLocator;
import io.micronaut.context.BeanProvider;
import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.Order;
Expand All @@ -27,7 +26,7 @@
import io.micronaut.http.bind.RequestBinderRegistry;
import io.micronaut.http.bind.binders.RequestArgumentBinder;
import io.micronaut.http.body.MessageBodyHandlerRegistry;
import io.micronaut.http.server.HttpServerConfiguration;
import io.micronaut.http.server.netty.configuration.NettyHttpServerConfiguration;
import io.micronaut.http.server.netty.multipart.MultipartBodyArgumentBinder;
import io.micronaut.http.server.netty.multipart.NettyStreamingFileUpload;
import io.micronaut.scheduling.TaskExecutors;
Expand All @@ -53,8 +52,7 @@ public final class NettyServerRequestBinderRegistry implements RequestBinderRegi

public NettyServerRequestBinderRegistry(ConversionService conversionService,
List<RequestArgumentBinder> binders,
BeanLocator beanLocator,
BeanProvider<HttpServerConfiguration> httpServerConfiguration,
BeanProvider<NettyHttpServerConfiguration> httpServerConfiguration,
@Named(TaskExecutors.BLOCKING)
BeanProvider<ExecutorService> executorService,
MessageBodyHandlerRegistry bodyHandlerRegistry) {
Expand All @@ -68,7 +66,6 @@ public NettyServerRequestBinderRegistry(ConversionService conversionService,
internalRequestBinderRegistry.addArgumentBinder(new NettyPublisherBodyBinder(
nettyBodyAnnotationBinder));
internalRequestBinderRegistry.addArgumentBinder(new MultipartBodyArgumentBinder(
beanLocator,
httpServerConfiguration
));
internalRequestBinderRegistry.addArgumentBinder(new NettyInputStreamBodyBinder());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,20 @@ public class NettyHttpServerConfiguration extends HttpServerConfiguration {
@SuppressWarnings("WeakerAccess")
public static final int DEFAULT_HTTP3_INITIAL_MAX_STREAMS_BIDIRECTIONAL = 100;

/**
* Default value for {@link #formMaxFields}.
*
* @since 4.6.0
*/
public static final int DEFAULT_FORM_MAX_FIELDS = 128;

/**
* Default value for {@link #formMaxBufferedBytes}.
*
* @since 4.6.0
*/
public static final int DEFAULT_FORM_MAX_BUFFERED_BYTES = 1024;

private static final Logger LOG = LoggerFactory.getLogger(NettyHttpServerConfiguration.class);

private final List<ChannelPipelineListener> pipelineCustomizers;
Expand Down Expand Up @@ -197,6 +211,8 @@ public class NettyHttpServerConfiguration extends HttpServerConfiguration {
private boolean eagerParsing = DEFAULT_EAGER_PARSING;
private int jsonBufferMaxComponents = DEFAULT_JSON_BUFFER_MAX_COMPONENTS;
private boolean legacyMultiplexHandlers = false;
private int formMaxFields = DEFAULT_FORM_MAX_FIELDS;
private int formMaxBufferedBytes = DEFAULT_FORM_MAX_BUFFERED_BYTES;

/**
* Default empty constructor.
Expand Down Expand Up @@ -764,6 +780,48 @@ public void setLegacyMultiplexHandlers(boolean legacyMultiplexHandlers) {
this.legacyMultiplexHandlers = legacyMultiplexHandlers;
}

/**
* The maximum number of form fields permitted in a request.
*
* @return The maximum number of form fields
* @since 4.6.0
*/
public int getFormMaxFields() {
return formMaxFields;
}

/**
* The maximum number of form fields permitted in a request.
*
* @param formMaxFields The maximum number of form fields
* @since 4.6.0
*/
public void setFormMaxFields(int formMaxFields) {
this.formMaxFields = formMaxFields;
}

/**
* The maximum number of bytes the form / multipart decoders are allowed to buffer internally.
* This sets a limit on form field size.
*
* @return The maximum number of buffered bytes
* @since 4.6.0
*/
public int getFormMaxBufferedBytes() {
return formMaxBufferedBytes;
}

/**
* The maximum number of bytes the form / multipart decoders are allowed to buffer internally.
* This sets a limit on form field size.
*
* @param formMaxBufferedBytes The maximum number of buffered bytes
* @since 4.6.0
*/
public void setFormMaxBufferedBytes(int formMaxBufferedBytes) {
this.formMaxBufferedBytes = formMaxBufferedBytes;
}

/**
* Http2 settings.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package io.micronaut.http.server.netty.multipart;

import io.micronaut.context.BeanLocator;
import io.micronaut.context.BeanProvider;
import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.NonNull;
Expand All @@ -24,12 +23,12 @@
import io.micronaut.http.HttpRequest;
import io.micronaut.http.bind.binders.NonBlockingBodyArgumentBinder;
import io.micronaut.http.multipart.CompletedPart;
import io.micronaut.http.server.HttpServerConfiguration;
import io.micronaut.http.server.multipart.MultipartBody;
import io.micronaut.http.server.netty.FormDataHttpContentProcessor;
import io.micronaut.http.server.netty.HttpContentProcessorAsReactiveProcessor;
import io.micronaut.http.server.netty.NettyHttpRequest;
import io.micronaut.http.server.netty.body.NettyByteBody;
import io.micronaut.http.server.netty.configuration.NettyHttpServerConfiguration;
import io.netty.handler.codec.http.DefaultHttpContent;
import io.netty.handler.codec.http.multipart.Attribute;
import io.netty.handler.codec.http.multipart.FileUpload;
Expand All @@ -49,15 +48,14 @@
*/
@Internal
public class MultipartBodyArgumentBinder implements NonBlockingBodyArgumentBinder<MultipartBody> {
private final BeanProvider<HttpServerConfiguration> httpServerConfiguration;
private final BeanProvider<NettyHttpServerConfiguration> httpServerConfiguration;

/**
* Default constructor.
*
* @param beanLocator The bean locator
* @param httpServerConfiguration The server configuration
*/
public MultipartBodyArgumentBinder(BeanLocator beanLocator, BeanProvider<HttpServerConfiguration> httpServerConfiguration) {
public MultipartBodyArgumentBinder(BeanProvider<NettyHttpServerConfiguration> httpServerConfiguration) {
this.httpServerConfiguration = httpServerConfiguration;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package io.micronaut.http.server.netty

import io.micronaut.context.ApplicationContext
import io.micronaut.context.annotation.Requires
import io.micronaut.core.type.Argument
import io.micronaut.http.HttpRequest
import io.micronaut.http.HttpResponse
import io.micronaut.http.HttpStatus
import io.micronaut.http.MediaType
import io.micronaut.http.annotation.Body
import io.micronaut.http.annotation.Consumes
import io.micronaut.http.annotation.Controller
import io.micronaut.http.annotation.Post
import io.micronaut.http.client.HttpClient
import io.micronaut.http.server.multipart.MultipartBody
import io.micronaut.runtime.server.EmbeddedServer
import io.micronaut.scheduling.TaskExecutors
import io.micronaut.scheduling.annotation.ExecuteOn
import reactor.core.publisher.Flux
import spock.lang.Specification

class FormLimitSpec extends Specification {
def 'field count limit'(boolean multipart, int toSend, int limitToConfigure) {
given:
def ctx = ApplicationContext.run([
'spec.name' : 'FormLimitSpec',
'micronaut.server.port' : -1,
'micronaut.server.netty.form-max-fields' : limitToConfigure,
'micronaut.http.client.exception-on-error-status': false
])
def server = ctx.getBean(EmbeddedServer)
server.start()
def client = ctx.createBean(HttpClient, server.URI).toBlocking()

def body

if (multipart) {
def builder = io.micronaut.http.client.multipart.MultipartBody.builder()
for (int i = 0; i < toSend; i++) {
builder.addPart("f" + i, "val")
}
body = builder.build()
} else {
StringBuilder builder = new StringBuilder()
for (int i = 0; i < toSend; i++) {
if (builder.length() > 0) builder.append('&')
builder.append('f').append(i).append('=').append('val')
}
body = builder.toString()
}

when:
HttpResponse<String> response = client.exchange(HttpRequest.POST("/form-limit", body)
.contentType(multipart ? MediaType.MULTIPART_FORM_DATA_TYPE : MediaType.APPLICATION_FORM_URLENCODED_TYPE), Argument.STRING, Argument.STRING)
then:
if (toSend > limitToConfigure) {
assert response.status() == HttpStatus.REQUEST_ENTITY_TOO_LARGE
} else {
assert response.body() == "attributes: " + toSend
}

where:
multipart | toSend | limitToConfigure
false | 100 | 128
false | 100 | 64
true | 100 | 128
true | 100 | 64
}

def 'field name length limit'(boolean multipart, int toSend, int limitToConfigure) {
given:
def ctx = ApplicationContext.run([
'spec.name' : 'FormLimitSpec',
'micronaut.server.port' : -1,
'micronaut.server.netty.form-max-buffered-bytes' : limitToConfigure,
'micronaut.http.client.exception-on-error-status': false
])
def server = ctx.getBean(EmbeddedServer)
server.start()
def client = ctx.createBean(HttpClient, server.URI).toBlocking()

def body = " " * toSend

when:
HttpResponse<String> response = client.exchange(HttpRequest.POST("/form-limit", body)
.contentType(multipart ? MediaType.MULTIPART_FORM_DATA + ";boundary=abcdef" : MediaType.APPLICATION_FORM_URLENCODED), Argument.STRING, Argument.STRING)
then:
if (toSend > limitToConfigure) {
assert response.status() == HttpStatus.REQUEST_ENTITY_TOO_LARGE
} else {
assert response.body() == "attributes: 0"
}

where:
multipart | toSend | limitToConfigure
false | 100 | 128
false | 100 | 64
/* client does not support raw multipart requests atm
true | 100 | 128
true | 100 | 64
*/
}

@Controller("/form-limit")
@Requires(property = "spec.name", value = "FormLimitSpec")
static class MyCtrl {
@Post
@ExecuteOn(TaskExecutors.BLOCKING)
@Consumes([MediaType.APPLICATION_FORM_URLENCODED, MediaType.MULTIPART_FORM_DATA])
String post(@Body MultipartBody body) {
return "attributes: " + Flux.from(body).doOnNext { it.getBytes() }.count().block()
}
}
}

0 comments on commit 8f0a7ab

Please sign in to comment.