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

Incorrect buffer size check in Convert.TryToHexString / Convert.TryToHexStringLower #110094

Closed
alexanderkozlenko opened this issue Nov 22, 2024 · 2 comments
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Comments

@alexanderkozlenko
Copy link
Contributor

Description

Affected methods:

  • Convert.TryToHexString(ReadOnlySpan<byte>, Span<char>, out int)
  • Convert.TryToHexStringLower(ReadOnlySpan<byte>, Span<char>, out int)

Problem:

The expression destination.Length > source.Length * 2 in both implementations should have inverted comparison operator.

if (source.Length == 0)
{
charsWritten = 0;
return true;
}
else if (source.Length > int.MaxValue / 2 || destination.Length > source.Length * 2)
{
charsWritten = 0;
return false;
}

if (source.Length == 0)
{
charsWritten = 0;
return true;
}
else if (source.Length > int.MaxValue / 2 || destination.Length > source.Length * 2)
{
charsWritten = 0;
return false;
}

Reproduction Steps

var src = new byte[] { 0xa1, 0xb2, 0xc3, 0xd4 };

var dst7u = new char[7]; var dst8u = new char[8]; var dst9u = new char[9];
var dst7l = new char[7]; var dst8l = new char[8]; var dst9l = new char[9];

var res7u = Convert.TryToHexString(src, dst7u, out var chw7u);
var res8u = Convert.TryToHexString(src, dst8u, out var chw8u);
var res9u = Convert.TryToHexString(src, dst9u, out var chw9u);
var res7l = Convert.TryToHexStringLower(src, dst7l, out var chw7l);
var res8l = Convert.TryToHexStringLower(src, dst8l, out var chw8l);
var res9l = Convert.TryToHexStringLower(src, dst9l, out var chw9l);

Console.WriteLine($"7u: {res7u}, {chw7u}, '{new(dst7u)}'");
Console.WriteLine($"8u: {res8u}, {chw8u}, '{new(dst8u)}'");
Console.WriteLine($"9u: {res9u}, {chw9u}, '{new(dst9u)}'");
Console.WriteLine($"7l: {res7l}, {chw7l}, '{new(dst7l)}'");
Console.WriteLine($"8l: {res8l}, {chw8l}, '{new(dst8l)}'");
Console.WriteLine($"9l: {res9l}, {chw9l}, '{new(dst9l)}'");

Expected behavior

7u: False, 0, ''
8u: True, 8, 'A1B2C3D4'
9u: True, 8, 'A1B2C3D4'
7l: False, 0, ''
8l: True, 8, 'a1b2c3d4'
9l: True, 8, 'a1b2c3d4'

Actual behavior

7u: True, 8, 'A1B2C3D'
8u: True, 8, 'A1B2C3D4'
9u: False, 0, ''
7l: True, 8, 'a1b2c3d'
8l: True, 8, 'a1b2c3d4'
9l: False, 0, ''

Regression?

No response

Known Workarounds

Ensure destination.Length == source.Length * 2 before calling the affected methods.

Configuration

.NET Runtime 9.0.0

Other information

No response

@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 Nov 22, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 22, 2024
@KeterSCP
Copy link

Duplicate of #109807

@vcsjones
Copy link
Member

Closing as a duplicate #109807.

@vcsjones vcsjones closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

No branches or pull requests

3 participants