Follow-up from #1334, which introduced TrustedProxyOptions. Not a regression in that PR's behaviour — it's a gap in the new config surface, raised separately so it doesn't widen a security PR.
The mismatch
ForwardedHeadersOptions.ForwardLimit is int? (aspnetcore release/10.0), where null means unlimited:
public int? ForwardLimit { get; set; } = 1;
TrustedProxyOptions.ForwardLimit is a plain int with no validation, passed straight through. So "unlimited" is unexpressible, and two bad values are accepted silently.
Measured, not inferred
Instantiating the real ForwardedHeadersMiddleware and calling ApplyForwarders directly:
| config |
result |
ForwardLimit = -1, no forwarded headers at all |
System.OverflowException |
ForwardLimit = 0, X-Forwarded-For present |
no exception; RemoteIpAddress unchanged (processing silently off) |
Negative is the sharp edge. In ApplyForwarders, entryCount starts at 0 and there is no early return when no forwarded headers are present:
if (_options.ForwardLimit.HasValue && entryCount > _options.ForwardLimit)
{
entryCount = _options.ForwardLimit.Value;
}
var sets = new SetOfForwarders[entryCount];
0 > -1 is true, so entryCount becomes -1 and the allocation throws. Since UseForwardedHeaders() sits after UseExceptionHandler() in UseHeroPlatform, the effect is HTTP 500 on 100% of traffic — including requests carrying no forwarded headers at all — rather than a startup failure that would be caught in a deploy smoke test.
-1 is a plausible thing for an operator to write when reaching for "unlimited hops", precisely because the underlying framework option supports exactly that via null.
Zero is the quiet one. entryCount > 0 truncates to 0, the consume loop runs zero iterations, and forwarded headers stop being processed with no error and no log — reinstating the original API-02 symptom (proxy IP in rate-limit partitions and audit trails) while the config still reads as configured.
Suggested fix
Smallest: reject <= 0 in AddHeroPlatform's forwarded-headers registration with a message naming the setting, matching the treatment KnownProxies / KnownNetworks already get:
if (trustedProxy.ForwardLimit <= 0)
{
throw new InvalidOperationException(
$"TrustedProxyOptions:ForwardLimit must be at least 1; got {trustedProxy.ForwardLimit}.");
}
Larger, if expressing "unlimited" is wanted: make the property int? to mirror the framework, treat null as unlimited, and still reject <= 0. That changes the options contract, so it's worth a deliberate call rather than folding into #1334.
Happy to send whichever you prefer.
Follow-up from #1334, which introduced
TrustedProxyOptions. Not a regression in that PR's behaviour — it's a gap in the new config surface, raised separately so it doesn't widen a security PR.The mismatch
ForwardedHeadersOptions.ForwardLimitisint?(aspnetcorerelease/10.0), wherenullmeans unlimited:TrustedProxyOptions.ForwardLimitis a plainintwith no validation, passed straight through. So "unlimited" is unexpressible, and two bad values are accepted silently.Measured, not inferred
Instantiating the real
ForwardedHeadersMiddlewareand callingApplyForwardersdirectly:ForwardLimit = -1, no forwarded headers at allSystem.OverflowExceptionForwardLimit = 0,X-Forwarded-ForpresentRemoteIpAddressunchanged (processing silently off)Negative is the sharp edge. In
ApplyForwarders,entryCountstarts at0and there is no early return when no forwarded headers are present:0 > -1is true, soentryCountbecomes-1and the allocation throws. SinceUseForwardedHeaders()sits afterUseExceptionHandler()inUseHeroPlatform, the effect is HTTP 500 on 100% of traffic — including requests carrying no forwarded headers at all — rather than a startup failure that would be caught in a deploy smoke test.-1is a plausible thing for an operator to write when reaching for "unlimited hops", precisely because the underlying framework option supports exactly that vianull.Zero is the quiet one.
entryCount > 0truncates to0, the consume loop runs zero iterations, and forwarded headers stop being processed with no error and no log — reinstating the original API-02 symptom (proxy IP in rate-limit partitions and audit trails) while the config still reads as configured.Suggested fix
Smallest: reject
<= 0inAddHeroPlatform's forwarded-headers registration with a message naming the setting, matching the treatmentKnownProxies/KnownNetworksalready get:Larger, if expressing "unlimited" is wanted: make the property
int?to mirror the framework, treatnullas unlimited, and still reject<= 0. That changes the options contract, so it's worth a deliberate call rather than folding into #1334.Happy to send whichever you prefer.