Buffer overflow in ecc_get_key

[ulikos]

Prerequisites

• [x] Checked the developer manual
• [x] Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=repo%3Alibtom%2Flibtomcrypt
• [x] Checked that your issue isn't due to the fact that you're using asymmetric cryptography and you forgot linking in and/or setting an MPI provider (usually this causes either random crashes or runtime errors like LTC_ARGCHK 'ltc_mp.name != NULL' failure ...). c.f. Ch. "Math Descriptors" of the developer manual.
• [x] Checked that your issue isn't related to TomsFastMath's limitation that PK operations can by default only be done with max. 2048bit keys

Description

If *outlen is less than size, the buffer out is anyway written, because the assignment *outlen = size happens before the comparison size > *outlen.

Steps to Reproduce

Use a 160bit curve, put protection bytes around the out buffer, make the out buffer only 10 bytes:

    uint8_t protection1[] { 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa };
    uint8_t keybuffer[10] {};
    uint8_t protection2[] { 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa };
    unsigned long keylen {};
    keylen = sizeof(keybuffer);
    if (ecc_get_key(keybuffer, &keylen, PK_PRIVATE, &key) != CRYPT_OK) return -1;
    return 0;

0 is returned and protection1 is overridden!

Version

v1.18.2-717-g1e629e6f

Additional Information

Suggested patch:

diff --git a/src/pk/ecc/ecc_get_key.c b/src/pk/ecc/ecc_get_key.c
index d30fd068..891a7413 100644
--- a/src/pk/ecc/ecc_get_key.c
+++ b/src/pk/ecc/ecc_get_key.c
@@ -33,8 +33,11 @@ int ecc_get_key(unsigned char *out, unsigned long *outlen, int type, const ecc_k
    }
    else if (type == PK_PRIVATE) {
       if (key->type != PK_PRIVATE)                                                return CRYPT_PK_TYPE_MISMATCH;
+      if (size > *outlen) {
+         *outlen = size;
+         return CRYPT_BUFFER_OVERFLOW;
+      }
       *outlen = size;
-      if (size > *outlen)                                                         return CRYPT_BUFFER_OVERFLOW;
       if ((ksize = mp_unsigned_bin_size(key->k)) > size)                          return CRYPT_BUFFER_OVERFLOW;
       /* pad and store k */
       if ((err = mp_to_unsigned_bin(key->k, out + (size - ksize))) != CRYPT_OK)   return err;

[sjaeckel]

Thanks for reporting the issue and providing a fix!