From 429223d198aabacd129cf6dde5a4203b5af41737 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Mon, 10 Apr 2017 22:01:05 +0200 Subject: [PATCH] Fix x_int64.c Clearing a misunderstanding. The routines c2i_uint64_int() and i2c_uint64_int() expect to receive that internal values are absolute and with a separate sign flag, and the x_int64.c code handles values that aren't absolute and have the sign bit embedded. We therefore need to convert between absolute and non-absolute values for the encoding of negative values to be correct. [extended tests] Reviewed-by: Andy Polyakov (Merged from https://github.com/openssl/openssl/pull/3160) --- crypto/asn1/x_int64.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/crypto/asn1/x_int64.c b/crypto/asn1/x_int64.c index 33e4061699..63a32774bd 100644 --- a/crypto/asn1/x_int64.c +++ b/crypto/asn1/x_int64.c @@ -52,8 +52,11 @@ static int uint64_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, && utmp == 0) return -1; if ((it->size & INTxx_FLAG_SIGNED) == INTxx_FLAG_SIGNED - && (int64_t)utmp < 0) + && (int64_t)utmp < 0) { + /* i2c_uint64_int() assumes positive values */ + utmp = 0 - utmp; neg = 1; + } return i2c_uint64_int(cont, utmp, neg); } @@ -76,6 +79,9 @@ static int uint64_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, ASN1err(ASN1_F_UINT64_C2I, ASN1_R_TOO_LARGE); return 0; } + if (neg) + /* c2i_uint64_int() returns positive values */ + utmp = 0 - utmp; memcpy(cp, &utmp, sizeof(utmp)); return 1; } @@ -116,12 +122,22 @@ static int uint32_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, && utmp == 0) return -1; if ((it->size & INTxx_FLAG_SIGNED) == INTxx_FLAG_SIGNED - && (int32_t)utmp < 0) + && (int32_t)utmp < 0) { + /* i2c_uint64_int() assumes positive values */ + utmp = 0 - utmp; neg = 1; + } return i2c_uint64_int(cont, (uint64_t)utmp, neg); } +/* + * Absolute value of INT32_MIN: we can't just use -INT32_MIN as it produces + * overflow warnings. + */ + +#define ABS_INT32_MIN ((uint32_t)INT32_MAX + 1) + static int uint32_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype, char *free_cont, const ASN1_ITEM *it) { @@ -136,13 +152,20 @@ static int uint32_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, ASN1err(ASN1_F_UINT32_C2I, ASN1_R_ILLEGAL_NEGATIVE_VALUE); return 0; } - utmp2 = (uint32_t)utmp; - if (utmp != utmp2 - || ((it->size & INTxx_FLAG_SIGNED) == INTxx_FLAG_SIGNED - && !neg && utmp2 > INT32_MAX)) { - ASN1err(ASN1_F_UINT32_C2I, ASN1_R_TOO_LARGE); - return 0; + if (neg) { + if (utmp > ABS_INT32_MIN) { + ASN1err(ASN1_F_UINT32_C2I, ASN1_R_TOO_SMALL); + return 0; + } + utmp = 0 - utmp; + } else { + if (((it->size & INTxx_FLAG_SIGNED) != 0 && utmp > INT32_MAX) + || ((it->size & INTxx_FLAG_SIGNED) == 0 && utmp > UINT32_MAX)) { + ASN1err(ASN1_F_UINT32_C2I, ASN1_R_TOO_LARGE); + return 0; + } } + utmp2 = (uint32_t)utmp; memcpy(cp, &utmp2, sizeof(utmp2)); return 1; } -- 2.25.1