Fix overflow in AccessFailedAsync when DefaultLockoutTimeSpan is large#67647
Open
GOVINSAGA wants to merge 1 commit into
Open
Fix overflow in AccessFailedAsync when DefaultLockoutTimeSpan is large#67647GOVINSAGA wants to merge 1 commit into
GOVINSAGA wants to merge 1 commit into
Conversation
UserManager.AccessFailedAsync computed the lockout end date as UtcNow().Add(DefaultLockoutTimeSpan), which throws ArgumentOutOfRangeException when the resulting DateTimeOffset would exceed DateTimeOffset.MaxValue (for example when DefaultLockoutTimeSpan is set to TimeSpan.MaxValue to lock an account out indefinitely). Clamp the lockout end date to DateTimeOffset.MaxValue when the configured timespan would overflow. The overflow check subtracts the current time from DateTimeOffset.MaxValue (always a valid TimeSpan) to avoid overflowing while performing the check itself. Adds a regression test to the shared UserManager specification suite so all store implementations are covered. Fixes dotnet#60181 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Thanks for your PR, @GOVINSAGA. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #60181.
UserManager.AccessFailedAsyncsets the lockout end date with:When
DefaultLockoutTimeSpanis large enough that the result exceedsDateTimeOffset.MaxValue, this throwsArgumentOutOfRangeException. A legitimate scenario is settingDefaultLockoutTimeSpan = TimeSpan.MaxValueto lock an account out indefinitely (e.g. after repeated failed MFA attempts). Today that crashes instead of locking the user out.Change
Clamp the computed lockout end date to
DateTimeOffset.MaxValuewhen the configured timespan would overflow:The overflow check computes headroom as
DateTimeOffset.MaxValue - now(always a validTimeSpan), so the check itself cannot overflow.Tests
Adds
LockoutWithMaxTimeSpanDoesNotOverflowto the sharedUserManagerSpecificationTestBasesuite, so every store implementation (in-memory, EF Core) is covered. It setsDefaultLockoutTimeSpan = TimeSpan.MaxValue, triggers a lockout viaAccessFailedAsync, and asserts the call succeeds and the lockout end date isDateTimeOffset.MaxValue.