test for issue #507 always passes

[jamuir]

Description

There is a test vector for issue #507 in tests/der_test.c. However, if you revert the fix for #507, the test still passes (so the test in its current form doesn't really tell us much).

Steps to Reproduce

One way is to reset to d2027d6, revert 25c26a3 and then run the tests.

Another way is to revert the fix for #507 manually:

diff --git a/src/pk/asn1/der/utf8/der_decode_utf8_string.c b/src/pk/asn1/der/utf8/der_decode_utf8_string.c
index 93a5e5ed..c2b6bb6b 100644
--- a/src/pk/asn1/der/utf8/der_decode_utf8_string.c
+++ b/src/pk/asn1/der/utf8/der_decode_utf8_string.c
@@ -77,7 +77,7 @@ int der_decode_utf8_string(const unsigned char *in,  unsigned long inlen,
       for (z = 0; (tmp & 0x80) && (z <= 4); z++, tmp = (tmp << 1) & 0xFF);
 
       /* z should be in {0,2,3,4} */
-      if (z == 1 || z > 4) {
+      if (z > 4) {
          return CRYPT_INVALID_PACKET;
       }
 
@@ -85,7 +85,7 @@ int der_decode_utf8_string(const unsigned char *in,  unsigned long inlen,
       tmp >>= z;
 
       /* now update z so it equals the number of additional bytes to read */
-      if (z > 0) { --z; }
+      if (z > 1) { --z; }
 
       if (x + z > inlen) {
          return CRYPT_INVALID_PACKET;

To run the der test, I do this:

CFLAGS="-DUSE_LTM -DLTM_DESC -I../libtommath" EXTRALIBS="../libtommath/libtommath.a" make test && ./test der

[jamuir]

The test vector for #507 is expected to cause a failure (i.e. fail is good).

The reason it still fails when the fix is reverted/removed is because the ASN1 parser encounters invalid data when it reads beyond the test vector (typically it will read a byte that is not a valid ASN1 type id).

[jamuir]

I think the primary vulnerability uncovered in #507 is not in utf-8 decoding; it is the underflow of the unsigned computation "*inlen -= len;" inside src/pk/asn1/der/sequence/der_decode_sequence_flexi.c.

There should be an underflow check added inside der_decode_sequence_flexi.c. I have a change ready that does that, but we still need a good test for #507. This is why I opened the current issue.