PKCS10: check for null/empty extension request value

2 files changed, 58 insertions, 18 deletions

diff --git a/crypto/src/pkcs/Pkcs10CertificationRequest.cs b/crypto/src/pkcs/Pkcs10CertificationRequest.cs

index 9256b91f3..dbaaa34f6 100644
--- a/crypto/src/pkcs/Pkcs10CertificationRequest.cs
+++ b/crypto/src/pkcs/Pkcs10CertificationRequest.cs
@@ -487,41 +487,50 @@ namespace Org.BouncyCastle.Pkcs
                     try
                     {
                         attributePkcs = AttributePkcs.GetInstance(item);
-
                     }
                     catch (ArgumentException ex)
                     {
                         throw new ArgumentException("encountered non PKCS attribute in extensions block", ex);
                     }
 
-                    if (attributePkcs.AttrType.Equals(PkcsObjectIdentifiers.Pkcs9AtExtensionRequest))
+                    if (PkcsObjectIdentifiers.Pkcs9AtExtensionRequest.Equals(attributePkcs.AttrType))
                     {
                         X509ExtensionsGenerator generator = new X509ExtensionsGenerator();
 
-                        Asn1Sequence extensionSequence = Asn1Sequence.GetInstance(attributePkcs.AttrValues[0]);
+                        var attrValues = attributePkcs.AttrValues;
+                        if (attrValues == null || attrValues.Count == 0)
+                            throw new InvalidOperationException("pkcs_9_at_extensionRequest present but has no value");
 
+                        Asn1Sequence extensionSequence = Asn1Sequence.GetInstance(attrValues[0]);
 
-                        foreach (Asn1Encodable seqItem in extensionSequence)
+                        try
                         {
-
-                            Asn1Sequence itemSeq = Asn1Sequence.GetInstance(seqItem);
-                            if (itemSeq.Count == 2)
+                            foreach (Asn1Encodable seqItem in extensionSequence)
                             {
-                                generator.AddExtension(DerObjectIdentifier.GetInstance(itemSeq[0]), false, Asn1OctetString.GetInstance(itemSeq[1]).GetOctets());
-                            }
-                            else if (itemSeq.Count == 3)
-                            {
-                                generator.AddExtension(DerObjectIdentifier.GetInstance(itemSeq[0]), DerBoolean.GetInstance(itemSeq[1]).IsTrue, Asn1OctetString.GetInstance(itemSeq[2]).GetOctets());
-                            }
-                            else
-                            {
-                                throw new ArgumentException("incorrect sequence size of X509Extension got " + itemSeq.Count + " expected 2 or 3");
+                                Asn1Sequence itemSeq = Asn1Sequence.GetInstance(seqItem);
+
+                                if (itemSeq.Count == 2)
+                                {
+                                    generator.AddExtension(DerObjectIdentifier.GetInstance(itemSeq[0]), false, Asn1OctetString.GetInstance(itemSeq[1]).GetOctets());
+                                }
+                                else if (itemSeq.Count == 3)
+                                {
+                                    bool critical = DerBoolean.GetInstance(itemSeq[1]).IsTrue;
+                                    generator.AddExtension(DerObjectIdentifier.GetInstance(itemSeq[0]), critical, Asn1OctetString.GetInstance(itemSeq[2]).GetOctets());
+                                }
+                                else
+                                {
+                                    throw new InvalidOperationException("incorrect sequence size of X509Extension got " + itemSeq.Count + " expected 2 or 3");
+                                }
                             }
                         }
+                        catch (ArgumentException e)
+                        {
+                            throw new InvalidOperationException("asn1 processing issue: " + e.Message, e);
+                        }
 
                         return generator.Generate();
                     }
-
                 }
             }
 
diff --git a/crypto/test/src/pkcs/test/PKCS10Test.cs b/crypto/test/src/pkcs/test/PKCS10Test.cs

index 949045c8a..45371b8b4 100644
--- a/crypto/test/src/pkcs/test/PKCS10Test.cs
+++ b/crypto/test/src/pkcs/test/PKCS10Test.cs
@@ -9,6 +9,7 @@ using Org.BouncyCastle.Asn1;
 using Org.BouncyCastle.Asn1.X509;
 using Org.BouncyCastle.Asn1.Pkcs;
 using Org.BouncyCastle.Crypto;
+using Org.BouncyCastle.Utilities.Encoders;
 using Org.BouncyCastle.Utilities.Test;
 using Org.BouncyCastle.Security;
 
@@ -18,12 +19,43 @@ namespace Org.BouncyCastle.Pkcs.Tests
     public class Pkcs10Test
         : SimpleTest
     {
+        private static readonly byte[] EmptyExtensionsReq = Base64.Decode(
+                "MIICVDCCATwCAQAwADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKy8\n" +
+                "4oC/QPFkRBE04LIA5njEulZx/EEh+J2spnThoRwk+oycYEVKp95NSfGTAoNjTwUv\n" +
+                "TdB9c1PCPE1DmgZIVLEVvouB7sZbMbLSI0d//oMO/Wr/CZmvjPGB8DID7RJs0eqO\n" +
+                "gLgSuyBVrwbcSKtxH4NrNDsS5IZXCcE3xzkxMDdz72m9jvIrl2ivi+YmJ7cJo3N+\n" +
+                "DBEqHZW28oytOmVo+8zhxvnHb9w26GJEOxN5zYbiIVW2vU9OfeF9te+Rhnks43Pk\n" +
+                "YDDP2U4hR7q0BYrdkeWdA1ReleYyn/haeAoIVLZMANIOXobiqASKqSusVq9tLD67\n" +
+                "7TAywl5AVq8GOBzlXZUCAwEAAaAPMA0GCSqGSIb3DQEJDjEAMA0GCSqGSIb3DQEB\n" +
+                "CwUAA4IBAQAXck62gJw1deVOLVFAwBNVNXgJarHtDg3pauHTHvN+pSbdOTe1aRzb\n" +
+                "Tt4/govtuuGZsGWlUqiglLpl6qeS7Pe9m+WJwhH5yXnJ3yvy2Lc/XkeVQ0kt8uFg\n" +
+                "30UyrgKng6LDgUGFjDSiFr3dK8S/iYpDu/qpl1bWJPWmfmnIXzZWWvBdUTKlfoD9\n" +
+                "/NLIWINEzHQIBXGy2uLhutYOvDq0WDGOgtdFC8my/QajaJh5lo6mM/PlmcYjK286\n" +
+                "EdGSIxdME7hoW/ljA5355S820QZDkYx1tI/Y/YaY5KVOntwfDQzQiwWZ2PtpTqSK\n" +
+                "KYe2Ujb362yaERCE13DJC4Us9j8OOXcW\n");
+
         public override string Name
         {
 			get { return "Pkcs10"; }
         }
 
         [Test]
+        public void EmptyExtRequest()
+        {
+            Pkcs10CertificationRequest req = new Pkcs10CertificationRequest(EmptyExtensionsReq);
+
+            try
+            {
+                req.GetRequestedExtensions();
+                Fail("no exception thrown");
+            }
+            catch (InvalidOperationException e)
+            {
+                Assert.AreEqual("pkcs_9_at_extensionRequest present but has no value", e.Message);
+            }
+        }
+
+        [Test]
         public void BrokenRequestWithDuplicateExtension()
         {
             string keyName = "RSA";
@@ -123,7 +155,6 @@ namespace Org.BouncyCastle.Pkcs.Tests
             }
         }
 
-
         public override void PerformTest()
         {
             IAsymmetricCipherKeyPairGenerator pGen = GeneratorUtilities.GetKeyPairGenerator("RSA");