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

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

Closed
leechristensen opened this issue Jun 11, 2021 · 4 comments
Labels
bug Bug. An issue exist in our code. fixed-vNext

Comments

@leechristensen
Copy link

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
Copy link
Collaborator

Crypt32 commented Jun 11, 2021

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 added a commit to PKISolutions/Asn1DerParser.NET that referenced this issue Jun 12, 2021
Crypt32 added a commit to PKISolutions/Asn1DerParser.NET that referenced this issue Jun 12, 2021
@Crypt32
Copy link
Collaborator

Crypt32 commented Jun 12, 2021

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

  • I've updated Asn1ObjectIdentifier to handle really big numbers, which are larger than unsigned QWORDs
  • Updated Asn1Utils.EncodeObjectIdentifier and Asn1Utils.DecodeObjectIdentifier to use Asn1ObjectIdentifier class for encoding and decoding and which doesn't use .NET shortcuts (like CryptoConfig or X509EnhancedKeyUsageExtension classes). Instead, it relies on a pure math and BigInteger to represent OID arcs.

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.

Crypt32 added a commit that referenced this issue Jun 12, 2021
@Crypt32 Crypt32 added bug Bug. An issue exist in our code. fixed-vNext labels Jun 12, 2021
@leechristensen
Copy link
Author

Awesome! I appreciate the really fast response!

@Crypt32
Copy link
Collaborator

Crypt32 commented Jun 15, 2023

fixed in v4.0.1

@Crypt32 Crypt32 closed this as completed Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug. An issue exist in our code. fixed-vNext
Projects
None yet
Development

No branches or pull requests

2 participants