HMACSHA256 randomly throws: System.ArgumentException

[Ziuan]

Description

When using HMACSHA256.ComputeHash to sign some content, we randomly started to get the error described. The code supplied is heavily simplified to illustrate the problem.

The problem only occurs randomly, when building with Release settings, usually around iteration 10.000.
The non simplified version of the code usually crashes sooner.

We have verified that the problem exists on different computers (macOS and windows).

Using LLDB (macOS) we can see that one thread seems to be handling an unhandled exception pointing in the direction of a memory issue:

thread #2
    frame #0: 0x00007ff80645ca2e libsystem_kernel.dylib`mach_msg2_trap + 10
    frame #1: 0x00007ff80646ae3a libsystem_kernel.dylib`mach_msg2_internal + 84
    frame #2: 0x00007ff806463b62 libsystem_kernel.dylib`mach_msg_overwrite + 653
    frame #3: 0x00007ff80645cd1f libsystem_kernel.dylib`mach_msg + 19
    frame #4: 0x0000000100d02eb8 libcoreclr.dylib`MachMessage::Receive(unsigned int) + 72
    frame #5: 0x0000000100d02122 libcoreclr.dylib`SEHExceptionThread(void*) + 82
    frame #6: 0x00007ff80649c202 libsystem_pthread.dylib`_pthread_start + 99
    frame #7: 0x00007ff806497bab libsystem_pthread.dylib`thread_start + 15

Reproduction Steps

using System.Security.Cryptography;

namespace Identity.Tests;

[TestFixture]
public class SymmetricTests
{
    [Test]
    public void SymmetricEncryptor_Bounds()
    {
        for (var i = 0; i < 100000; i++)
        {
            var signedToken = Enumerable.Repeat((byte)0x20, i).ToArray();
            var ticket = Encrypt(signedToken);
            Assert.That(ticket, Is.Not.Null);
        }
    }

    private static byte[] MergeArrays(int additionalCapacity = 0, params byte[][] arrays)
    {
        var merged = new byte[arrays.Sum(a => a.Length) + additionalCapacity];
        var mergeIndex = 0;
        foreach (var array in arrays) {
            Array.Copy(array, 0, merged, mergeIndex, array.Length);
            mergeIndex += array.Length;
        }
        return merged;
    }

    private static byte[] Encrypt(byte[] toEncrypt)
    {
        const int signatureByteSize = 32;

        var someBytes = Enumerable.Repeat((byte)0x20, 64).ToArray();

        var result = MergeArrays(
            additionalCapacity: signatureByteSize,
            someBytes, toEncrypt);

        using var hmac = new HMACSHA256(someBytes);
        var payloadToSignLength = result.Length - signatureByteSize;
        hmac.ComputeHash(result, 0, payloadToSignLength);

        return result;
    }
}

Expected behavior

One should be able to call Array.Copy, HMACSHA256.ComputeHash, and other framework code without any memory violations.

Actual behavior

System.ArgumentException : Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection.

thrown from code: hmac.ComputeHash(result, 0, payloadToSignLength);

Regression?

The issue started happening after migration to .NET8 from .NET6.

Known Workarounds

No known workarounds.

Configuration

Code is running on .NET8 (8.0.202)
System 1:
Windows 11, x64, Ryzen 7950x

System 2:
Apple MacBook Pro, OSX 14.3.1, Intel i9 9980HK

Other information

May be related to calls to GC.KeepAlive(...) calls in the function-calls.

[vcsjones]

I can reproduce it, and only in a release build. I can also reproduce it without HMACSHA256, so I don't believe it to be an issue in System.Security. I can also reproduce it without NUnit.

Running the following code as an application in a release build will throw. It will not throw in a debug build.

for (var i = 0; i < 100000; i++)
{
    var signedToken = Enumerable.Repeat((byte)0x20, i).ToArray();
    var ticket = Encrypt(signedToken);
}

static byte[] MergeArrays(int additionalCapacity = 0, params byte[][] arrays)
{
    var merged = new byte[arrays.Sum(a => a.Length) + additionalCapacity];
    var mergeIndex = 0;
    foreach (var array in arrays) {
        Array.Copy(array, 0, merged, mergeIndex, array.Length);
        mergeIndex += array.Length;
    }
    return merged;
}

static byte[] Encrypt(byte[] toEncrypt)
{
    const int signatureByteSize = 32;

    var someBytes = Enumerable.Repeat((byte)0x20, 64).ToArray();

    var result = MergeArrays(
        additionalCapacity: signatureByteSize,
        someBytes, toEncrypt);

    var payloadToSignLength = result.Length - signatureByteSize;
    ComputeHash(result, 0, payloadToSignLength);

    return result;
}

static byte[] ComputeHash(byte[] buffer, int offset, int count)
{
    ArgumentNullException.ThrowIfNull(buffer);

    ArgumentOutOfRangeException.ThrowIfNegative(offset);
    if (count < 0 || (count > buffer.Length))
        throw new ArgumentException();
    if ((buffer.Length - count) < offset)
        throw new ArgumentException();

    // Return a fake HMAC.
    return new byte[32];
}

This was run with .NET 8.0.3 on ARM64 / Apple Silicon.

[vcsjones]

I am not an expert in diagnosing JIT related issues, but given the different behavior between a release build and a debug build, I think that is the next place to start looking.

[vcsjones]

With DOTNET_TieredCompilation=0, I can no longer reproduce it, so it certainly seems related to the JIT.

cc @dotnet/jit-contrib

[EgorBo]

From a quick look (and slightly modified repro) it is not PGO, OSR and R2R related

[EgorBo]

Repros with TC=0 as well. Looks like assertprop is the culprit

[EgorBo]

Looks like (buffer.Length - count) < offset) expression gets a constant VN and then folded into true.

[EgorBo]

Re-opening for .NET 8.0 backport considiration

[Ziuan]

The suggestion to add <TieredPGO>false</TieredPGO> or <TieredCompilation>false</TieredCompilation> sadly did not solve the problem.

[vcsjones]

If you are able to make a change to your code, it seems that HMACSHA256.HashData is not affected by the JIT issue.

Replace

using var hmac = new HMACSHA256(someBytes);
var payloadToSignLength = result.Length - signatureByteSize;
hmac.ComputeHash(result, 0, payloadToSignLength);

with

var payloadToSignLength = result.Length - signatureByteSize;
HMACSHA256.HashData(someBytes, result.AsSpan(0, payloadToSignLength));

This may be a performance improvement since this is no longer creating an HMACSHA256 object instance.

[Ziuan]

We solved our issue by using another overload of the function:

public byte[] ComputeHash(byte[] buffer)

instead of the previously used function:

public byte[] ComputeHash(byte[] buffer, int offset, int count)

Thanks for your reply regardless. Given the presence of #100968 I guess a backport is underway to resolve the underlying issue.

[EgorBo]

Fixed and backported to 8.0. Thanks for the repro!