Merge pull request #2278 from SparkiDev/cert_asn1

Better length checks when parsing ASN.1 certificates

wolfcrypt/src/asn.c

@@ -239,6 +239,24 @@ static int GetASNHeader(const byte* input, byte tag, word32* inOutIdx, int* len,
     return GetASNHeader_ex(input, tag, inOutIdx, len, maxIdx, 1);
 }
 
+static int GetHeader(const byte* input, byte* tag, word32* inOutIdx, int* len,
+                     word32 maxIdx, int check)
+{
+    word32 idx = *inOutIdx;
+    int    length;
+
+    if ((idx + 1) > maxIdx)
+        return BUFFER_E;
+
+    *tag = input[idx++];
+
+    if (GetLength_ex(input, &idx, &length, maxIdx, check) < 0)
+        return ASN_PARSE_E;
+
+    *len      = length;
+    *inOutIdx = idx;
+    return length;
+}
 
 WOLFSSL_LOCAL int GetSequence(const byte* input, word32* inOutIdx, int* len,
                            word32 maxIdx)
@@ -4293,12 +4311,12 @@ static int GetCertHeader(DecodedCert* cert)
 
 #if !defined(NO_RSA)
 /* Store Rsa Key, may save later, Dsa could use in future */
-static int StoreRsaKey(DecodedCert* cert)
+static int StoreRsaKey(DecodedCert* cert, word32 bitStringEnd)
 {
     int    length;
     word32 recvd = cert->srcIdx;
 
-    if (GetSequence(cert->source, &cert->srcIdx, &length, cert->maxIdx) < 0)
+    if (GetSequence(cert->source, &cert->srcIdx, &length, bitStringEnd) < 0)
         return ASN_PARSE_E;
 
     recvd = cert->srcIdx - recvd;
@@ -4366,7 +4384,7 @@ static int GetKey(DecodedCert* cert)
                     return ret;
             #endif
 
-            return StoreRsaKey(cert);
+            return StoreRsaKey(cert, cert->srcIdx + length);
         }
 
     #endif /* NO_RSA */
@@ -4653,8 +4671,8 @@ static int GetName(DecodedCert* cert, int nameType)
 #endif
 
     while (cert->srcIdx < (word32)length) {
-        byte   b;
+        byte   b = 0;
-        byte   joint[2];
+        byte   joint[3];
         byte   tooBig = FALSE;
         int    oidSz;
 
@@ -4678,16 +4696,15 @@ static int GetName(DecodedCert* cert, int nameType)
         /* v1 name types */
         if (joint[0] == 0x55 && joint[1] == 0x04) {
             const char*  copy = NULL;
-            int    strLen;
+            int    strLen = 0;
             byte   id;
 
-            cert->srcIdx += 2;
+            cert->srcIdx += 3;
-            id = cert->source[cert->srcIdx++];
+            id = joint[2];
-            b  = cert->source[cert->srcIdx++]; /* encoding */
+            if (GetHeader(cert->source, &b, &cert->srcIdx, &strLen,
-
+                          cert->maxIdx, 1) < 0) {
-            if (GetLength(cert->source, &cert->srcIdx, &strLen,
-                          cert->maxIdx) < 0)
                 return ASN_PARSE_E;
+            }
 
             if ( (strLen + 14) > (int)(ASN_NAME_MAX - idx)) {
                 /* include biggest pre fix header too 4 = "/serialNumber=" */

wolfcrypt/test/test.c

@@ -9225,9 +9225,119 @@ static const CertName certDefaultName = {
 
 #ifndef NO_RSA
 
-#if defined(WOLFSSL_TEST_CERT) && !defined(NO_FILESYSTEM)
+#if defined(WOLFSSL_TEST_CERT)
+static byte minSerial[] = { 0x02, 0x01, 0x01 };
+static byte minName[] = { 0x30, 0x00 };
+static byte nameBad[] = {
+    0x30, 0x08,
+          0x31, 0x06,
+                0x30, 0x04,
+                      0x06, 0x02,
+                            0x55, 0x04,
+};
+static byte minDates[] = {
+    0x30, 0x1e,
+          0x17, 0x0d,
+                0x31, 0x38, 0x30, 0x34, 0x31, 0x33, 0x31, 0x35,
+                0x32, 0x33, 0x31, 0x30, 0x5a,
+          0x17, 0x0d,
+                0x32, 0x31, 0x30, 0x31, 0x30, 0x37, 0x31, 0x35,
+                0x32, 0x33, 0x31, 0x30, 0x5a
+};
+static byte minPubKey[] = {
+    0x30, 0x1b,
+          0x30, 0x0d,
+                0x06, 0x09,
+                      0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01,
+                      0x01,
+                0x05, 0x00,
+          0x03, 0x0b,
+                0x00, 0x30, 0x08,
+                            0x02, 0x01,
+                                  0x03,
+                            0x02, 0x03,
+                                  0x01, 0x00, 0x01
+};
+static byte minSigAlg[] = {
+    0x30, 0x0d,
+          0x06, 0x09,
+                0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01,
+                0x0b,
+          0x05, 0x00
+};
+static byte minSig[] = {
+    0x03, 0x01,
+          0x00
+};
+
+static int add_seq(byte* certData, int offset, byte* data, byte length)
+{
+    XMEMMOVE(certData + offset + 2, data, length);
+    certData[offset++] = 0x30;
+    certData[offset++] = length;
+    return offset + length;
+}
+static int add_data(byte* certData, int offset, byte* data, byte length)
+{
+    XMEMCPY(certData + offset, data, length);
+    return offset + length;
+}
+
+static int cert_asn1_test(void)
+{
+    int ret;
+    int len[3];
+    DecodedCert cert;
+    byte certData[106];
+    byte* badCert = NULL;
+
+    len[2] = add_data(certData, 0, minSerial, (byte)sizeof(minSerial));
+    len[2] = add_data(certData, len[2], minSigAlg, (byte)sizeof(minSigAlg));
+    len[2] = add_data(certData, len[2], minName, (byte)sizeof(minName));
+    len[2] = add_data(certData, len[2], minDates, (byte)sizeof(minDates));
+    len[2] = add_data(certData, len[2], minName, (byte)sizeof(minName));
+    len[2] = add_data(certData, len[2], minPubKey, (byte)sizeof(minPubKey));
+    len[1] = add_seq(certData, 0, certData, len[2]);
+    len[1] = add_data(certData, len[1], minSigAlg, (byte)sizeof(minSigAlg));
+    len[1] = add_data(certData, len[1], minSig, (byte)sizeof(minSig));
+    len[0] = add_seq(certData, 0, certData, len[1]);
+
+    /* Minimal good certificate */
+    InitDecodedCert(&cert, certData, len[0], 0);
+    ret = ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL);
+    FreeDecodedCert(&cert);
+    if (ret != 0) {
+        ERROR_OUT(-6630, done);
+    }
+
+    /* Bad issuer name */
+    len[2] = add_data(certData, 0, minSerial, (byte)sizeof(minSerial));
+    len[2] = add_data(certData, len[2], minSigAlg, (byte)sizeof(minSigAlg));
+    len[2] = add_data(certData, len[2], nameBad, (byte)sizeof(nameBad));
+    len[1] = add_seq(certData, 0, certData, len[2]);
+    len[0] = add_seq(certData, 0, certData, len[1]);
+    /* Put data into allocated buffer to allow access error checking. */
+    badCert = (byte*)XMALLOC(len[0], HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+    XMEMCPY(badCert, certData, len[0]);
+    InitDecodedCert(&cert, badCert, len[0], 0);
+    ret = ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL);
+    FreeDecodedCert(&cert);
+    if (ret != ASN_PARSE_E) {
+        ERROR_OUT(-6631, done);
+    }
+    XFREE(badCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+    badCert = NULL;
+    ret = 0;
+
+done:
+    if (badCert != NULL)
+        XFREE(badCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+    return ret;
+}
+
 int cert_test(void)
 {
+#if !defined(NO_FILESYSTEM)
     DecodedCert cert;
     byte*       tmp;
     size_t      bytes;
@@ -9276,10 +9386,14 @@ int cert_test(void)
 done:
     FreeDecodedCert(&cert);
     XFREE(tmp, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+#endif /* !NO_FILESYSTEM */
+
+    if (ret == 0)
+        ret = cert_asn1_test();
 
     return ret;
 }
-#endif /* WOLFSSL_TEST_CERT && !NO_FILESYSTEM */
+#endif /* WOLFSSL_TEST_CERT */
 
 #if defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_TEST_CERT) && \
    !defined(NO_FILESYSTEM)