Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible stack overflow in TotalOrderIeee754Comparer #86593

Merged
merged 2 commits into from
May 23, 2023

Conversation

vcsjones
Copy link
Member

If an (incorrectly implemented) T for TotalOrderIeee754Comparer returns a negative value for GetSignificandByteCount, this will stack overflow.

This changes the check to treat the value as unsigned. In doing so, that causes the negative value to go in to to new byte path instead of the stackalloc byte path, which results in a runtime exception instead of a stack overflow, which takes down the process.

@ghost ghost assigned vcsjones May 22, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 22, 2023
@vcsjones vcsjones added area-System.Numerics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 22, 2023
@ghost
Copy link

ghost commented May 22, 2023

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

Issue Details

If an (incorrectly implemented) T for TotalOrderIeee754Comparer returns a negative value for GetSignificandByteCount, this will stack overflow.

This changes the check to treat the value as unsigned. In doing so, that causes the negative value to go in to to new byte path instead of the stackalloc byte path, which results in a runtime exception instead of a stack overflow, which takes down the process.

Author: vcsjones
Assignees: vcsjones
Labels:

area-System.Numerics

Milestone: -

@stephentoub stephentoub merged commit 8f19f4e into dotnet:main May 23, 2023
@vcsjones vcsjones deleted the total-order-overflow branch May 23, 2023 17:12
@ghost ghost locked as resolved and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants