Change ValidateDer to iterative evaluation (#128874)

Since this change doesn't eagerly dive into CONSTRUCTED values, it will
change when it encounters a value that gets rejected, but since it
doesn't report where the invalid value was found, that doesn't matter.

Author: bartonjs

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
@@ -272,54 +273,84 @@ internal static void DisposeAll(this IEnumerable<IDisposable> disposables)
             }
         }
 
-        internal static void ValidateDer(ReadOnlySpan<byte> encodedValue)
+        internal static unsafe void ValidateDer(ReadOnlySpan<byte> encodedValue)
         {
             try
             {
-                Asn1Tag tag;
-                ValueAsnReader reader = new ValueAsnReader(encodedValue, AsnEncodingRules.DER);
+                const int StackQueueSize = 16;
+                Span<(int Offset, int Length)> stack = stackalloc (int, int)[StackQueueSize];
+                int stackCount = 0;
 
-                while (reader.HasData)
+                stack[stackCount++] = (0, encodedValue.Length);
+
+                do
                 {
-                    tag = reader.PeekTag();
+                    (int offset, int length) = stack[--stackCount];
+
+                    ValueAsnReader reader = new ValueAsnReader(
+                        encodedValue.Slice(offset, length),
+                        AsnEncodingRules.DER);
 
-                    // If the tag is in the UNIVERSAL class
-                    //
-                    // DER limits the constructed encoding to SEQUENCE and SET, as well as anything which gets
-                    // a defined encoding as being an IMPLICIT SEQUENCE.
-                    if (tag.TagClass == TagClass.Universal)
+                    while (reader.HasData)
                     {
-                        switch ((UniversalTagNumber)tag.TagValue)
+                        Asn1Tag tag = reader.PeekTag();
+
+                        // If the tag is in the UNIVERSAL class
+                        //
+                        // DER limits the constructed encoding to SEQUENCE and SET, as well as anything which gets
+                        // a defined encoding as being an IMPLICIT SEQUENCE.
+                        if (tag.TagClass == TagClass.Universal)
+                        {
+                            switch ((UniversalTagNumber)tag.TagValue)
+                            {
+                                case UniversalTagNumber.External:
+                                case UniversalTagNumber.Embedded:
+                                case UniversalTagNumber.Sequence:
+                                case UniversalTagNumber.Set:
+                                case UniversalTagNumber.UnrestrictedCharacterString:
+                                    if (!tag.IsConstructed)
+                                    {
+                                        throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding);
+                                    }
+
+                                    break;
+                                default:
+                                    if (tag.IsConstructed)
+                                    {
+                                        throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding);
+                                    }
+
+                                    break;
+                            }
+                        }
+
+                        if (tag.IsConstructed)
                         {
-                            case UniversalTagNumber.External:
-                            case UniversalTagNumber.Embedded:
-                            case UniversalTagNumber.Sequence:
-                            case UniversalTagNumber.Set:
-                            case UniversalTagNumber.UnrestrictedCharacterString:
-                                if (!tag.IsConstructed)
+                            ReadOnlySpan<byte> content = reader.PeekContentBytes();
+
+                            if (content.Length > 0)
+                            {
+                                if (!encodedValue.Overlaps(content, out int contentOffset))
                                 {
-                                    throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding);
+                                    Debug.Fail("Contents do not overlap original span");
+                                    throw new CryptographicException();
                                 }
 
-                                break;
-                            default:
-                                if (tag.IsConstructed)
+                                if (stackCount == stack.Length)
                                 {
-                                    throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding);
+                                    Span<(int, int)> nextStack = new (int, int)[stack.Length * 2];
+                                    stack.CopyTo(nextStack);
+                                    stack = nextStack;
                                 }
 
-                                break;
+                                stack[stackCount++] = (contentOffset, content.Length);
+                            }
                         }
-                    }
 
-                    if (tag.IsConstructed)
-                    {
-                        ValidateDer(reader.PeekContentBytes());
+                        // Skip past the current value.
+                        reader.ReadEncodedValue();
                     }
-
-                    // Skip past the current value.
-                    reader.ReadEncodedValue();
-                }
+                } while (stackCount > 0);
             }
             catch (AsnContentException e)
             {

src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestApiTests.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Formats.Asn1;
 using System.Security.Cryptography.SLHDsa.Tests;
 using System.Security.Cryptography.Tests;
 using Xunit;
@@ -513,6 +514,87 @@ public static void InvalidDerInAttribute()
             }
         }
 
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public static void DeeplyNestedOtherRequestAttribute(bool invalid)
+        {
+            byte[] encoded = BuildAttributeValue();
+
+            if (invalid)
+            {
+                encoded[^2] = 0x10;
+            }
+
+            using (ECDsa key = ECDsa.Create(EccTestData.Secp384r1Data.KeyParameters))
+            {
+                CertificateRequest req = new CertificateRequest(
+                    "CN=Test",
+                    key,
+                    HashAlgorithmName.SHA384);
+
+                req.OtherRequestAttributes.Add(
+                    new AsnEncodedData(
+                        new Oid("1.2.840.113549.1.9.7", null),
+                        encoded));
+
+                X509SignatureGenerator gen = X509SignatureGenerator.CreateForECDsa(key);
+
+                if (invalid)
+                {
+                    Assert.Throws<CryptographicException>(() => req.CreateSigningRequest());
+                    Assert.Throws<CryptographicException>(() => req.CreateSigningRequest(gen));
+                }
+                else
+                {
+                    req.CreateSigningRequest();
+                    req.CreateSigningRequest(gen);
+                }
+            }
+
+            static byte[] BuildAttributeValue()
+            {
+                const int TargetDepth = 7500;
+
+                // SEQUENCE-OF
+                //   SEQUENCE-OF x7500 deep
+                //      BOOLEAN FALSE
+                //   SEQUENCE-OF x7500 deep
+
+                // Each of the two really deep values would cause a StackOverflow, use two of them to ensure coverage
+                // of the deep validator queue reuse.
+
+                AsnWriter writer = new AsnWriter(AsnEncodingRules.DER, initialCapacity: 64 * 1024);
+
+                writer.PushSequence();
+
+                for (int i = 0; i < TargetDepth; i++)
+                {
+                    writer.PushSequence();
+                }
+
+                writer.WriteBoolean(false);
+
+                for (int i = 0; i < TargetDepth; i++)
+                {
+                    writer.PopSequence();
+                }
+
+                for (int i = 0; i < TargetDepth; i++)
+                {
+                    writer.PushSequence();
+                }
+
+                for (int i = 0; i < TargetDepth; i++)
+                {
+                    writer.PopSequence();
+                }
+
+                writer.PopSequence();
+                return writer.Encode();
+            }
+        }
+
         [Fact]
         public static void LoadNullArray()
         {