Skip to content

Conversation

geor-gen
Copy link

@geor-gen geor-gen commented Oct 3, 2025

As I understand, the length argument cannot be negative in practice, it is a cosmetic check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2025
Copy link
Contributor

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

@En3Tho
Copy link
Contributor

En3Tho commented Oct 3, 2025

I assume you've meant < short.MinValue ?

@geor-gen
Copy link
Author

geor-gen commented Oct 3, 2025

Yes, sorry, fixed it

@wfurt
Copy link
Member

wfurt commented Oct 3, 2025

short MinValue is -32768. That does not make much sense to me either. I could see the arguments that the length should be positive.

@geor-gen
Copy link
Author

geor-gen commented Oct 3, 2025

The change was originally intended to only make the overflow impossible. But enforcing the positive length is more logical, I will change it.

private static unsafe void SetField(ref MessageField field, int length, int offset)
{
if (length > short.MaxValue)
if (length > short.MaxValue || length < 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (length > short.MaxValue || length < 0)
if (length is < 0 or > short.MaxValue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Security community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants