asn1: Template CHOICE element 0 bug

While we no longer have a decoder CHOICE element 0 bug, we did still
have one encode and copy and free that was leading to a memory leak (and
_save trashing) prior to the fix for

    asn1: Fix 1-byte leaks in der_copy_octet_string()

This commit fixes that.

lib/asn1/check-gen.c

@@ -655,6 +655,12 @@ test_cert(void)
     size_t i;
     int ret;
 
+    memset(&c, 0, sizeof(c));
+    ret = copy_Certificate(&c, &c2);
+    if (ret)
+        return ret;
+    free_Certificate(&c2);
+
     for (i = 0; i < sizeof(certs)/sizeof(certs[0]); i++) {
 
 	ret = decode_Certificate((unsigned char *)certs[i].cert,
@@ -1114,6 +1120,57 @@ test_decorated(void)
     return 0;
 }
 
+static int
+test_extensible_choice(void)
+{
+    PA_FX_FAST_REQUEST r, r2;
+    size_t len, size;
+    void *ptr;
+    int ret;
+
+    memset(&r, 0, sizeof(r));
+
+    ret = copy_PA_FX_FAST_REQUEST(&r, &r2);
+    if (ret)
+        return ret;
+    free_PA_FX_FAST_REQUEST(&r2);
+
+    r.element = 0;
+    r.u.asn1_ellipsis.data = "hello";
+    r.u.asn1_ellipsis.length = sizeof("hello") - 1;
+    ret = copy_PA_FX_FAST_REQUEST(&r, &r2);
+    if (ret)
+        errx(1, "Out of memory");
+    if (r2.element != 0)
+        errx(1, "Extensible CHOICE copy failure to set discriminant to 0");
+    if (r2.u.asn1_ellipsis.length != r.u.asn1_ellipsis.length)
+        errx(1, "Extensible CHOICE copy failure to copy extension");
+    if (memcmp(r.u.asn1_ellipsis.data, r2.u.asn1_ellipsis.data,
+               r.u.asn1_ellipsis.length) != 0)
+        errx(1, "Extensible CHOICE copy failure to copy extension (2)");
+    free_PA_FX_FAST_REQUEST(&r2);
+
+    ASN1_MALLOC_ENCODE(PA_FX_FAST_REQUEST, ptr, len, &r, &size, ret);
+    if (ret || len != size)
+        errx(1, "Extensible CHOICE encoding failure");
+
+    ret = decode_PA_FX_FAST_REQUEST(ptr, len, &r2, &size);
+    if (ret || len != size)
+        errx(1, "Extensible CHOICE decoding failure");
+
+    if (r2.element != 0)
+        errx(1, "Extensible CHOICE decode failure to set discriminant to 0");
+    if (r2.u.asn1_ellipsis.length != r.u.asn1_ellipsis.length)
+        errx(1, "Extensible CHOICE decode failure to copy extension");
+    if (memcmp(r.u.asn1_ellipsis.data, r2.u.asn1_ellipsis.data,
+               r.u.asn1_ellipsis.length) != 0)
+        errx(1, "Extensible CHOICE decode failure to copy extension (2)");
+
+    free_PA_FX_FAST_REQUEST(&r2);
+    free(ptr);
+    return 0;
+}
+
 static int
 test_decorated_choice(void)
 {
@@ -2674,6 +2731,8 @@ main(int argc, char **argv)
 
     DO_ONE(test_default);
 
+    DO_ONE(test_extensible_choice);
+
     DO_ONE(test_decorated_choice);
     DO_ONE(test_decorated);
 

lib/asn1/template.c

@@ -1176,6 +1176,13 @@ _asn1_decode(const struct asn1_template *t, unsigned flags,
 		    return ret;
 	    }
 	    if (i >= A1_HEADER_LEN(choice) + 1 || !choice[i].tt) {
+                /*
+                 * If this is an extensible CHOICE, then choice->tt will be the
+                 * offset to u.ellipsis.  If it's not, then this "extension" is
+                 * an error and must stop parsing it.  (We could be permissive
+                 * and throw away the extension, though one might as well just
+                 * mark such a CHOICE as extensible.)
+                 */
 		if (choice->tt == 0)
 		    return ASN1_BAD_ID;
 
@@ -1786,16 +1793,20 @@ _asn1_encode(const struct asn1_template *t, unsigned char *p, size_t len, const
 	    }
 
 	    if (*element == 0) {
-		ret += der_put_octet_string(p, len,
+                if (choice->tt) {
-					    DPOC(data, choice->tt), &datalen);
+                    /* This is an extensible CHOICE */
+                    ret += der_put_octet_string(p, len,
+                                                DPOC(data, choice->tt), &datalen);
+                    len -= datalen; p -= datalen;
+                } /* else this is really an error -- XXX what to do? */
 	    } else {
 		choice += *element;
 		el = DPOC(data, choice->offset);
 		ret = _asn1_encode(choice->ptr, p, len, el, &datalen);
 		if (ret)
 		    return ret;
+                len -= datalen; p -= datalen;
 	    }
-	    len -= datalen; p -= datalen;
 
 	    break;
 	}
@@ -2175,7 +2186,8 @@ _asn1_length(const struct asn1_template *t, const void *data)
 		break;
 
 	    if (*element == 0) {
-		ret += der_length_octet_string(DPOC(data, choice->tt));
+                if (choice->tt)
+                    ret += der_length_octet_string(DPOC(data, choice->tt));
 	    } else {
 		choice += *element;
 		ret += _asn1_length(choice->ptr, DPOC(data, choice->offset));
@@ -2350,7 +2362,16 @@ _asn1_free(const struct asn1_template *t, void *data)
 		break;
 
 	    if (*element == 0) {
-		der_free_octet_string(DPO(data, choice->tt));
+                /*
+                 * If choice->tt != 0 then this is an extensible choice, and
+                 * the offset choice->tt is the offset to u.ellipsis.
+                 */
+                if (choice->tt != 0)
+                    der_free_octet_string(DPO(data, choice->tt));
+                /*
+                 * Else this was a not-fully initialized CHOICE.  We could
+                 * stand to memset clear the rest of it though...
+                 */
 	    } else {
 		choice += *element;
 		_asn1_free(choice->ptr, DPO(data, choice->offset));
@@ -2727,6 +2748,7 @@ _asn1_print(const struct asn1_template *t,
 	    if (*element > A1_HEADER_LEN(choice)) {
                 r = rk_strpoolprintf(r, "null");
             } else if (*element == 0) {
+                /* XXX If choice->tt then we should print the u.ellipsis */
                 r = rk_strpoolprintf(r, "null");
 	    } else {
 		choice += *element;
@@ -3024,7 +3046,12 @@ _asn1_copy(const struct asn1_template *t, const void *from, void *to)
 	    *telement = *felement;
 
 	    if (*felement == 0) {
-		ret = der_copy_octet_string(DPOC(from, choice->tt), DPO(to, choice->tt));
+                if (choice->tt)
+                    ret = der_copy_octet_string(DPOC(from, choice->tt), DPO(to, choice->tt));
+                /*
+                 * Else we should really memset clear the rest of this choice,
+                 * but we don't really know its size.
+                 */
 	    } else {
 		choice += *felement;
 		ret = _asn1_copy(choice->ptr,