Integer Overflow Causing HTTP1 Connections to Fail in Kestrel

[nerddtvg]

When Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions is configured with MaxRequestHeadersTotalSize as int.MaxValue, the assignment in Http1Connection.cs causes an integer overflow prior to conversion to long. This overflowed value is passed through to the header parser which uses the negative value as a string length parameter. This causes 500 errors and connections to fail.

aspnetcore/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs

Line 639 in bf50999

_remainingRequestHeadersBytesAllowed = ServerOptions.Limits.MaxRequestHeadersTotalSize + 2;

services.Configure<Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions>(options =>
{
  options.Limits.MaxRequestHeadersTotalSize = int.MaxValue;
});

_remainingRequestHeadersBytesAllowed = ServerOptions.Limits.MaxRequestHeadersTotalSize + 2;

Type int at its max value + 2 is -2147483647 after being set:

warn: Microsoft.AspNetCore.Server.Kestrel[0]
      Connection processing ended abnormally.
      System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'length')
         at System.ThrowHelper.ThrowStartOrEndArgumentValidationException(Int64 start)
         at System.Buffers.ReadOnlySequence`1.Slice(SequencePosition start, Int64 length)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.<TakeMessageHeaders>g__TrimAndTakeMessageHeaders|41_0(SequenceReader`1& reader, Boolean trailers)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TakeMessageHeaders(SequenceReader`1& reader, Boolean trailers)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.ParseRequest(SequenceReader`1& reader)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TryParseRequest(ReadResult result, Boolean& endConnection)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequestsAsync[TContext](IHttpApplication`1 application)
Microsoft.AspNetCore.Server.Kestrel: Warning: Connection processing ended abnormally.

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'length')
   at System.ThrowHelper.ThrowStartOrEndArgumentValidationException(Int64 start)
   at System.Buffers.ReadOnlySequence`1.Slice(SequencePosition start, Int64 length)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.<TakeMessageHeaders>g__TrimAndTakeMessageHeaders|41_0(SequenceReader`1& reader, Boolean trailers)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TakeMessageHeaders(SequenceReader`1& reader, Boolean trailers)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.ParseRequest(SequenceReader`1& reader)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TryParseRequest(ReadResult result, Boolean& endConnection)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequestsAsync[TContext](IHttpApplication`1 application)

[Tratcher]

Interesting find. What's the impact to your application? Setting MaxRequestHeadersTotalSize to MaxValue would be extremely dangerous in a real server, and I'd be surprised if you ever received a request remotely that large.

[nerddtvg]

I do agree it is a dangerous setting, this is only set in a development environment during testing.

The impact is simply that any HTTP 1.0 and HTTP 1.1 fails if that value is MaxValue. It doesn't matter the actual header length, because of the overflow, parsing any headers fails due to a negative length value in ReadOnlySequence.Slice.

Using HTTP/2 does not seem to fail.

[nerddtvg]

I appreciate the merge! Do you think this could be back ported to the 6.0 LTS tree? Should I submit a PR for that branch?

https://github.com/dotnet/aspnetcore/blob/release/6.0/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs#L640

[Tratcher]

This does not qualify for backporting since there is an easy workaround.