#59424 refactored parts of DataProtection to use newer crypto APIs that allowed Span<T> usage. However, it introduced 2 bugs in the CalculateAndValidateMac method for apps targeting < .NET 5+.
First:
|
correctHashArray = validationAlgorithm.ComputeHash(payloadArray, macOffset, eofOffset - macOffset); |
This is using the wrong offsets for computing the hash, it should be
ComputeHash(payloadArray, ivOffset, macOffset - ivOffset)
Second:
|
if (!CryptoUtil.TimeConstantBuffersAreEqual(correctHash, payloadMacSpan)) |
This is comparing against
correctHash which is assigned an array (or stackalloc) but never used, but we're writing the computed hash (see first code snippet) to a new array and assigning it to
correctHashArray. So we end up comparing something against nothing which always fails.
This is actually the saving grace, because this check always fails the scenario is just completely unusable instead of broken in a bad way.
#59424 refactored parts of DataProtection to use newer crypto APIs that allowed
Span<T>usage. However, it introduced 2 bugs in theCalculateAndValidateMacmethod for apps targeting< .NET 5+.First:
aspnetcore/src/DataProtection/DataProtection/src/Managed/ManagedAuthenticatedEncryptor.cs
Line 450 in e9e6afa
This is using the wrong offsets for computing the hash, it should be
ComputeHash(payloadArray, ivOffset, macOffset - ivOffset)Second:
aspnetcore/src/DataProtection/DataProtection/src/Managed/ManagedAuthenticatedEncryptor.cs
Line 454 in e9e6afa
This is comparing against
correctHashwhich is assigned an array (or stackalloc) but never used, but we're writing the computed hash (see first code snippet) to a new array and assigning it tocorrectHashArray. So we end up comparing something against nothing which always fails.This is actually the saving grace, because this check always fails the scenario is just completely unusable instead of broken in a bad way.