[openssl-dev] Heap corruption in asn1_item_ex_combine_new()

[Dr. Stephen Henson]

On Tue, Mar 31, 2015, Julien Kauffmann wrote:

> 
> Basically what happens is that, somewhere inside the call to
> PEM_write_bio_ECPrivateKey(), an ASN1 sequence of 3 elements is
> allocated. The corresponding code is as follow (in
> crypto/asn1/tasn_new.c:181):
> 
> if (!combine) {
>     *pval = OPENSSL_malloc(it->size);
>     if (!*pval)
>         goto memerr;
>     memset(*pval, 0, it->size);
>     asn1_do_lock(pval, 0, it);
>     asn1_enc_init(pval, it);
> }
> for (i = 0, tt = it->templates; i < it->tcount; tt++, i++) {
>     pseqval = asn1_get_field_ptr(pval, tt);
>     if (!ASN1_template_new(pseqval, tt))
>         goto memerr;
> }
> 
> In the sample I gave, at some point OPENSSL_malloc allocates 12
> bytes for a 3-elements ASN1 SEQUENCE. The for-loop then initializes
> every element: ASN1_template_new() calls in turn
> asn1_item_ex_combine_new() which, at line 103, does:
> 

Can you determine the value of it->sname where this happens? That will
indicate the precise structure where this is a problem. From your description
I'd say it is X9_62_PENTANOMIAL.

> if (!combine)
>     *pval = NULL;
> 
> The problem is that pval at this point points to an element of the
> SEQUENCE that is only 4 bytes long. On 64 bits systems this causes
> the next 4 bytes to be set to 0x00000000. For the first two elements
> of the sequence, this gets recovered by the next element being
> initialized. However for the last element, this affectation happens
> to write 4 bytes past the allocated memory, corrupting the heap.
> 

This should only be a problem where sizeof(long) < sizeof(ASN1_VALUE *) on
many 64 bit platforms they are the same. I'd guess this is 64 bit Windows
you're testing on?

> I'm not sure what is the best place to fix this. Do you have any hints ?
> 

I'll look into it.

In the meantime I'd suggest you add a call to use the named curve version of
EC parameters: this is only a problem with the explicit form which is very
rarely used but for historical compatibility reasons it is the default.
If you want to use the key for TLS you need the named curve form anyway.

To do this call:

EC_GROUP_set_asn1_flag(group, OPENSSL_EC_NAMED_CURVE);

Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org