SocketsHttpHandler.MaxResponseHeadersLength can cause internal 32-bit overflow with HTTP/2

[JamesNK]

Description

Noticed here: grpc/grpc-dotnet#1842 (comment)

MaxResponseHeadersLength is in kilobytes so internally multiplies the value by 1024.

However, the value it is stored on for HTTP/2 is 32 bit:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs

Line 88 in 08a11e4

private int _headerBudgetRemaining;

HTTP/3 is using 64 bit:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Line 41 in 80640d4

private long _headerBudgetRemaining;

Reproduction Steps

new SocketsHttpHandler()
  {
    MaxResponseHeadersLength = 20 * 1024 * 1024
  };

Then do some HTTP/2 requests.

Expected behavior

Either works, or provides a friendly argument exception when setting MaxResponseHeadersLength that the value exceeds the maximum size.

Actual behavior

Internal overflow of 32-bit value, unpredictable behavior, bad error message.

      System.Net.Http.HttpRequestException: An error occurred while sending the request.
       ---> System.IO.IOException: The request was aborted.
       ---> System.Net.Http.HttpRequestException: The HTTP response headers length exceeded the set limit of 21474836480 bytes.
         at System.Net.Http.Http2Connection.Http2Stream.OnHeader(ReadOnlySpan`1 name, ReadOnlySpan`1 value)
         at System.Net.Http.Http2Connection.Http2Stream.System.Net.Http.IHttpHeadersHandler.OnStaticIndexedHeader(Int32 index)
         at System.Net.Http.HPack.HPackDecoder.OnIndexedHeaderField(Int32 index, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.Parse(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.DecodeInternal(ReadOnlySpan`1 data, IHttpHeadersHandler handler)
         at System.Net.Http.Http2Connection.ProcessHeadersFrame(FrameHeader frameHeader)
         at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
         at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
         at System.Net.Http.Http2Connection.Http2Stream.TryEnsureHeaders()
         at System.Net.Http.Http2Connection.Http2Stream.ReadResponseHeadersAsync(CancellationToken cancellationToken)
         at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
         at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at Grpc.Shared.TelemetryHeaderHandler.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
         at Grpc.Net.Client.Balancer.Internal.BalancerHttpHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         at Grpc.Net.Client.Internal.GrpcCall`2.RunCall(HttpRequestMessage request, Nullable`1 timeout)

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Noticed here: grpc/grpc-dotnet#1842 (comment)

MaxResponseHeadersLength is in kilobytes so internally multiplies the value by 1024.

However, the value it is stored on for HTTP/2 is 32 bit:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs

Line 88 in 08a11e4

private int _headerBudgetRemaining;

HTTP/3 is using 64 bit:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Line 41 in 80640d4

private long _headerBudgetRemaining;

Reproduction Steps

new SocketsHttpHandler()
  {
    MaxResponseHeadersLength = 20 * 1024 * 1024
  };

Then do some HTTP/2 requests.

Expected behavior

Either works, or provides a friendly argument exception when setting MaxResponseHeadersLength that the value exceeds the maximum size.

Actual behavior

Internal overflow of 32-bit value, unpredictable behavior, bad error message.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author:	JamesNK
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	-

[MihaZupan]

Technically a bug, but if you're setting this so high you're pretty much begging to be DOSed.

[AnashOommen]

@MihaZupan could you kindly explain why a high value is begging to be DOSed? In Google Ads API (which uses grpc), we return serialized server errors to the caller as trailing metadata, and that data can be more than a few KB. How can a user DOS themselves in this scenario?

[MihaZupan]

A few KB is perfectly fine.
The issue is when you are telling the client that it's okay to receive tens of MBs or GBs of headers on a single response.
All of this data has to be buffered in memory (HttpResponseMessage.TrailingHeaders for example).

A malicious server could feed you GBs of data that the client would be forced to keep in memory all at once.

The main point for your use case is what @JamesNK mentioned in the gRPC issue:

MaxResponseHeadersLength is kilobytes so 20 megs (20 * 1024) is enough.

The fact that we have a bug here with the overflow should not block anyone as there's no reason to ever use a value this large.
If anything, it prevented you from setting the limit 1000x higher than you likely intended.

[AnashOommen]

@MihaZupan gotcha. Can we fix the underlying overflow by throwing an exception when trying to set a large value? The way this fails now isn't very helpful.

[MihaZupan]

I don't thing we'd want to start throwing now given that we've been silently handling it for HTTP/1.1 so far.

We would likely fix HTTP/2 to behave the same way as 1.1 (cap at int.MaxValue), and potentially consider an analyzer to warn you that the value being set doesn't make any sense.

[AnashOommen]

Yeah, capping + analyzer works fine as well.