PKISolutions / pkix.net

.NET PKI framework to extend cryptography support in CLR.
Microsoft Public License
47 stars 20 forks source link

CryptoConfig.EncodeOID Bug - "Value was either too large or too small for an Int32." #44

Closed leechristensen closed 1 year ago

leechristensen commented 3 years ago

Multiple places in the code and its dependent libraries end up calling .NET's CryptoConfig.EncodeOID:

SysadminsLV.Asn1Parser.Asn1Utils.EncodeObjectIdentifier uses it as well and it is used extensively throughout the code base: image

There appears to be an integer parsing bug in CryptoConfig.EncodeOID. The following PowerShell demonstrates this bug.

PS C:\> [System.Security.Cryptography.CryptoConfig]::EncodeOID('1.3.6.1.4.1.311.21.8.2473183039')

Exception calling "EncodeOID" with "1" argument(s): "Value was either too large or too small for an Int32."
At line:1 char:1
+ [System.Security.Cryptography.CryptoConfig]::EncodeOID('1.3.6.1.4.1.3 ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : OverflowException

The bug is due to the following code in .NET's CryptoConfig.EncodeOID function:

public static byte[] EncodeOID(string str)
{
    if (str == null)
    {
        throw new ArgumentNullException("str");
    }
    char[] separator = new char[]
    {
        '.'
    };
    string[] array = str.Split(separator);
    uint[] array2 = new uint[array.Length];
    for (int i = 0; i < array.Length; i++)
    {
        array2[i] = (uint)int.Parse(array[i], CultureInfo.InvariantCulture);
    }

Note that the code use splits the OID string by periods, and then attempts to parse each numeric value using int.Parse. The bug is 2473183039 is a valid OID value, but it is too large for int.Parse to handle. As such, the Value was either too large or too small for an Int32. exception gets thrown.

We originally encountered bug when using PSPKI's Get-CATemplate cmdlet in an environment where AD CS assigned a template OID that caused the exception. In that case, pkix.net appears to use CryptoConfig.EncodeOID primarly for OID input validation (just making sure it's a well-formed OID). I'd suggest implementing your own function to do the validation until .NET can fix the bug upstream. Other places in the code, however, appear to use SysadminsLV.Asn1Parser.Asn1Utils.EncodeObjectIdentifier to get the OID bytes, so a replacement for CryptoConfig.EncodeOID may be needed in those instances.

Crypt32 commented 3 years ago

Thanks for report, I will take a look into it. Stuff in SysadminsLV.Asn1Parser.Asn1Utils is mostly obsolete. There is more proper ASN.1 OBJECT_IDENTIFIER handler: https://github.com/PKISolutions/Asn1DerParser.NET/blob/master/Asn1Parser/Universal/Asn1ObjectIdentifier.cs it treats tokens as unsigned 64-bit QWORDs. But it may be reasonable to use BigInts for safety just in case if someone gets really mad and use enormous values. Either way, this needs to be reviewed and addressed.

Crypt32 commented 3 years ago

A follow-up. I've updated Asn1Parser library to address mentioned issues as follows:

Here is the test (in PowerShell):

PS C:\> $oid = New-Object SysadminsLV.Asn1Parser.Universal.Asn1ObjectIdentifier "1.2.999.18446744073709551615456"
PS C:\> [SysadminsLV.Asn1Parser.Asn1Utils]::DecodeObjectIdentifier($oid.RawData)

Value                           FriendlyName
-----                           ------------
1.2.999.18446744073709551615456

PS C:\> [SysadminsLV.Asn1Parser.Asn1Utils]::DecodeObjectIdentifier($oid.RawData)

Value                           FriendlyName
-----                           ------------
1.2.999.18446744073709551615456

I chose 18446744073709551615456 value which is larger than QWORD and encoder/decoder functions handle this.

leechristensen commented 3 years ago

Awesome! I appreciate the really fast response!

Crypt32 commented 1 year ago

fixed in v4.0.1