From 42bad9f5806d80ed7a9f46af8ff87b141dca98de Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Sun, 10 Jan 2016 21:37:17 +0100 Subject: [PATCH 1/2] fix for issue #58 - possible overflow in ecc_ansi_x963_export --- src/pk/ecc/ecc_ansi_x963_export.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/pk/ecc/ecc_ansi_x963_export.c b/src/pk/ecc/ecc_ansi_x963_export.c index 09dae07..f195a8e 100644 --- a/src/pk/ecc/ecc_ansi_x963_export.c +++ b/src/pk/ecc/ecc_ansi_x963_export.c @@ -32,7 +32,7 @@ int ecc_ansi_x963_export(ecc_key *key, unsigned char *out, unsigned long *outlen) { unsigned char buf[ECC_BUF_SIZE]; - unsigned long numlen; + unsigned long numlen, xlen, ylen; LTC_ARGCHK(key != NULL); LTC_ARGCHK(out != NULL); @@ -42,6 +42,12 @@ int ecc_ansi_x963_export(ecc_key *key, unsigned char *out, unsigned long *outlen return CRYPT_INVALID_ARG; } numlen = key->dp->size; + xlen = mp_unsigned_bin_size(key->pubkey.x); + ylen = mp_unsigned_bin_size(key->pubkey.y); + + if (xlen > numlen || ylen > numlen || sizeof(buf) < numlen) { + return CRYPT_BUFFER_OVERFLOW; + } if (*outlen < (1 + 2*numlen)) { *outlen = 1 + 2*numlen; @@ -53,12 +59,12 @@ int ecc_ansi_x963_export(ecc_key *key, unsigned char *out, unsigned long *outlen /* pad and store x */ zeromem(buf, sizeof(buf)); - mp_to_unsigned_bin(key->pubkey.x, buf + (numlen - mp_unsigned_bin_size(key->pubkey.x))); + mp_to_unsigned_bin(key->pubkey.x, buf + (numlen - xlen)); XMEMCPY(out+1, buf, numlen); /* pad and store y */ zeromem(buf, sizeof(buf)); - mp_to_unsigned_bin(key->pubkey.y, buf + (numlen - mp_unsigned_bin_size(key->pubkey.y))); + mp_to_unsigned_bin(key->pubkey.y, buf + (numlen - ylen)); XMEMCPY(out+1+numlen, buf, numlen); *outlen = 1 + 2*numlen; From 10e577e24a89b9cc9ac3543dab58ec7b0920405e Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 14 Jan 2016 21:32:33 +0100 Subject: [PATCH 2/2] there's no need to check out on function entry ...someone could then do something like this... unsigned char* out = NULL; unsigned long len = 0; while(ecc_ansi_x963_export(key, out, &len) == CRYPT_BUFFER_OVERFLOW && len == 0) { out = malloc(len); } ...as if someone would ever like to do something like that... --- src/pk/ecc/ecc_ansi_x963_export.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pk/ecc/ecc_ansi_x963_export.c b/src/pk/ecc/ecc_ansi_x963_export.c index f195a8e..e834c95 100644 --- a/src/pk/ecc/ecc_ansi_x963_export.c +++ b/src/pk/ecc/ecc_ansi_x963_export.c @@ -19,7 +19,7 @@ /** @file ecc_ansi_x963_export.c ECC Crypto, Tom St Denis -*/ +*/ #ifdef LTC_MECC @@ -35,7 +35,6 @@ int ecc_ansi_x963_export(ecc_key *key, unsigned char *out, unsigned long *outlen unsigned long numlen, xlen, ylen; LTC_ARGCHK(key != NULL); - LTC_ARGCHK(out != NULL); LTC_ARGCHK(outlen != NULL); if (ltc_ecc_is_valid_idx(key->idx) == 0) { @@ -54,6 +53,8 @@ int ecc_ansi_x963_export(ecc_key *key, unsigned char *out, unsigned long *outlen return CRYPT_BUFFER_OVERFLOW; } + LTC_ARGCHK(out != NULL); + /* store byte 0x04 */ out[0] = 0x04;