Merge pull request #533 from libtom/fix-373

really implement DER decoding resursion limit

Author: sjaeckel

src/pk/asn1/der/sequence/der_decode_sequence_flexi.c

@@ -39,11 +39,12 @@ static int _new_element(ltc_asn1_list **l)
    @param in      The input buffer
    @param inlen   [in/out] The length of the input buffer and on output the amount of decoded data
    @param out     [out] A pointer to the linked list
+   @param depth   The depth/level of decoding recursion we've already reached
    @return CRYPT_OK on success.
 */
-int der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc_asn1_list **out)
+static int _der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc_asn1_list **out, unsigned long depth)
 {
-   ltc_asn1_list *l, *t;
+   ltc_asn1_list *l;
    unsigned long err, identifier, len, totlen, data_offset, id_len, len_len;
    void          *realloc_tmp;
 
@@ -428,6 +429,12 @@ int der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc
                }
              }
 
+             /* check that we don't go over the recursion limit */
+             if (depth > LTC_DER_MAX_RECURSION) {
+                err = CRYPT_PK_ASN1_ERROR;
+                goto error;
+             }
+
              if ((l->data = XMALLOC(len)) == NULL) {
                 err = CRYPT_MEM;
                 goto error;
@@ -446,7 +453,7 @@ int der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc
              len_len = len;
 
              /* Sequence elements go as child */
-             if ((err = der_decode_sequence_flexi(in, &len, &(l->child))) != CRYPT_OK) {
+             if ((err = _der_decode_sequence_flexi(in, &len, &(l->child), depth+1)) != CRYPT_OK) {
                 goto error;
              }
              if (len_len != len) {
@@ -463,17 +470,6 @@ int der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc
                 l->child->parent = l;
              }
 
-             t = l;
-             len_len = 0;
-             while((t != NULL) && (t->child != NULL)) {
-                len_len++;
-                t = t->child;
-             }
-             if (len_len > LTC_DER_MAX_RECURSION) {
-                err = CRYPT_PK_ASN1_ERROR;
-                goto error;
-             }
-
              break;
 
          case 0x80: /* Context-specific */
@@ -535,6 +531,18 @@ int der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc
    return err;
 }
 
+/**
+   ASN.1 DER Flexi(ble) decoder will decode arbitrary DER packets and create a linked list of the decoded elements.
+   @param in      The input buffer
+   @param inlen   [in/out] The length of the input buffer and on output the amount of decoded data
+   @param out     [out] A pointer to the linked list
+   @return CRYPT_OK on success.
+*/
+int der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc_asn1_list **out)
+{
+   return _der_decode_sequence_flexi(in, inlen, out, 0);
+}
+
 #endif
 
 

tests/der_test.c

@@ -1560,21 +1560,27 @@ static void der_toolong_test(void)
 
 static void _der_recursion_limit(void)
 {
-   int failed = 0;
-   unsigned int n;
+   unsigned int n, m;
    unsigned long integer = 123, s;
    ltc_asn1_list seqs[LTC_DER_MAX_RECURSION + 2], dummy[1], *flexi;
    unsigned char buf[2048];
-   LTC_SET_ASN1(dummy, 0, LTC_ASN1_SHORT_INTEGER, &integer, 1);
-   LTC_SET_ASN1(seqs, LTC_DER_MAX_RECURSION + 1, LTC_ASN1_SEQUENCE, dummy, 1);
-   for (n = 0; n < LTC_DER_MAX_RECURSION + 1; ++n) {
-      LTC_SET_ASN1(seqs, LTC_DER_MAX_RECURSION - n, LTC_ASN1_SEQUENCE, &seqs[LTC_DER_MAX_RECURSION - n + 1], 1);
+   for (m = 0; m < 3; ++m) {
+      LTC_SET_ASN1(dummy, 0, LTC_ASN1_SHORT_INTEGER, &integer, 1);
+      LTC_SET_ASN1(seqs, LTC_DER_MAX_RECURSION + 1, LTC_ASN1_SEQUENCE, dummy, 1);
+      for (n = m; n < LTC_DER_MAX_RECURSION + 1; ++n) {
+         LTC_SET_ASN1(seqs, LTC_DER_MAX_RECURSION - n, LTC_ASN1_SEQUENCE, &seqs[LTC_DER_MAX_RECURSION - n + 1], 1);
+      }
+      s = sizeof(buf);
+      DO(der_encode_sequence(&seqs[m], 1, buf, &s));
+      DO(der_decode_sequence(buf, s, &seqs[m], 1));
+      if (m < 2) {
+         SHOULD_FAIL(der_decode_sequence_flexi(buf, &s, &flexi));
+      }
+      else {
+         DO(der_decode_sequence_flexi(buf, &s, &flexi));
+         der_free_sequence_flexi(flexi);
+      }
    }
-   s = sizeof(buf);
-   DO(der_encode_sequence(seqs, 1, buf, &s));
-   DO(der_decode_sequence(buf, s, seqs, 1));
-   SHOULD_FAIL(der_decode_sequence_flexi(buf, &s, &flexi));
-   if (failed) exit(EXIT_FAILURE);
 }
 
 int der_test(void)