From: Pauli Date: Thu, 6 Jul 2017 01:39:03 +0000 (+1000) Subject: Memory bounds checking in asn1 code. X-Git-Tag: OpenSSL_1_1_1-pre1~1099 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=60eba30f60de55e3c782469fa555eede82606099;hp=eee9552212ecc9e19bc09ea8a1b8428dc7394f45 Memory bounds checking in asn1 code. Check that sprint, strcpy don't overflow. Avoid some strlen operations when the previous sprintf return value can be used. Also fix the undefined behaviour `*(long *)x = y` when x isn't a long or character pointer. ISO/IEC 9899:1999 6.5/7 for the details. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3869) --- diff --git a/crypto/asn1/a_gentm.c b/crypto/asn1/a_gentm.c index 2c5fb1c3d2..5cfc3ff15e 100644 --- a/crypto/asn1/a_gentm.c +++ b/crypto/asn1/a_gentm.c @@ -1,5 +1,5 @@ /* - * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -207,7 +207,7 @@ ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, char *p; struct tm *ts; struct tm data; - size_t len = 20; + const size_t len = 20; ASN1_GENERALIZEDTIME *tmps = NULL; if (s == NULL) @@ -237,10 +237,10 @@ ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s, tmps->data = (unsigned char *)p; } - sprintf(p, "%04d%02d%02d%02d%02d%02dZ", ts->tm_year + 1900, - ts->tm_mon + 1, ts->tm_mday, ts->tm_hour, ts->tm_min, - ts->tm_sec); - tmps->length = strlen(p); + tmps->length = BIO_snprintf(p, len, "%04d%02d%02d%02d%02d%02dZ", + ts->tm_year + 1900, ts->tm_mon + 1, + ts->tm_mday, ts->tm_hour, ts->tm_min, + ts->tm_sec); tmps->type = V_ASN1_GENERALIZEDTIME; #ifdef CHARSET_EBCDIC_not ebcdic2ascii(tmps->data, tmps->data, tmps->length); diff --git a/crypto/asn1/a_mbstr.c b/crypto/asn1/a_mbstr.c index 46764b292c..e644fe0542 100644 --- a/crypto/asn1/a_mbstr.c +++ b/crypto/asn1/a_mbstr.c @@ -1,5 +1,5 @@ /* - * Copyright 1999-2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1999-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -100,14 +100,14 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len, if ((minsize > 0) && (nchar < minsize)) { ASN1err(ASN1_F_ASN1_MBSTRING_NCOPY, ASN1_R_STRING_TOO_SHORT); - sprintf(strbuf, "%ld", minsize); + BIO_snprintf(strbuf, sizeof(strbuf), "%ld", minsize); ERR_add_error_data(2, "minsize=", strbuf); return -1; } if ((maxsize > 0) && (nchar > maxsize)) { ASN1err(ASN1_F_ASN1_MBSTRING_NCOPY, ASN1_R_STRING_TOO_LONG); - sprintf(strbuf, "%ld", maxsize); + BIO_snprintf(strbuf, sizeof(strbuf), "%ld", maxsize); ERR_add_error_data(2, "maxsize=", strbuf); return -1; } diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c index f0ec42f71c..fc78e309a6 100644 --- a/crypto/asn1/a_time.c +++ b/crypto/asn1/a_time.c @@ -1,5 +1,5 @@ /* - * Copyright 1999-2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1999-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -81,17 +81,20 @@ ASN1_GENERALIZEDTIME *ASN1_TIME_to_generalizedtime(const ASN1_TIME *t, goto done; } - /* grow the string */ + /* + * Grow the string by two bytes. + * The actual allocation is t->length + 3 to include a terminator byte. + */ if (!ASN1_STRING_set(ret, NULL, t->length + 2)) goto err; str = (char *)ret->data; /* Work out the century and prepend */ - if (t->data[0] >= '5') - strcpy(str, "19"); - else - strcpy(str, "20"); - - strcat(str, (const char *)t->data); + memcpy(str, t->data[0] >= '5' ? "19" : "20", 2); + /* + * t->length + 1 is the size of the data and the allocated buffer has + * this much space after the first two characters. + */ + OPENSSL_strlcpy(str + 2, (const char *)t->data, t->length + 1); done: if (out != NULL && *out == NULL) diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c index 25393ee152..5a4b1742f7 100644 --- a/crypto/asn1/a_utctm.c +++ b/crypto/asn1/a_utctm.c @@ -1,5 +1,5 @@ /* - * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -21,7 +21,7 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d) int n, i, l, o, min_l = 11, strict = 0; if (d->type != V_ASN1_UTCTIME) - return (0); + return 0; l = d->length; a = (char *)d->data; o = 0; @@ -150,9 +150,9 @@ int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str) return 0; s->type = V_ASN1_UTCTIME; } - return (1); - } else - return (0); + return 1; + } + return 0; } ASN1_UTCTIME *ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t t) @@ -166,7 +166,7 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t, char *p; struct tm *ts; struct tm data; - size_t len = 20; + const size_t len = 20; int free_s = 0; if (s == NULL) { @@ -199,15 +199,14 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t, s->data = (unsigned char *)p; } - sprintf(p, "%02d%02d%02d%02d%02d%02dZ", ts->tm_year % 100, - ts->tm_mon + 1, ts->tm_mday, ts->tm_hour, ts->tm_min, - ts->tm_sec); - s->length = strlen(p); + s->length = BIO_snprintf(p, len, "%02d%02d%02d%02d%02d%02dZ", + ts->tm_year % 100, ts->tm_mon + 1, ts->tm_mday, + ts->tm_hour, ts->tm_min, ts->tm_sec); s->type = V_ASN1_UTCTIME; #ifdef CHARSET_EBCDIC_not ebcdic2ascii(s->data, s->data, s->length); #endif - return (s); + return s; err: if (free_s) ASN1_UTCTIME_free(s); @@ -272,10 +271,9 @@ int ASN1_UTCTIME_print(BIO *bp, const ASN1_UTCTIME *tm) if (BIO_printf(bp, "%s %2d %02d:%02d:%02d %d%s", _asn1_mon[M - 1], d, h, m, s, y + 1900, (gmt) ? " GMT" : "") <= 0) - return (0); - else - return (1); + return 0; + return 1; err: BIO_write(bp, "Bad time value", 14); - return (0); + return 0; } diff --git a/crypto/asn1/asn1_par.c b/crypto/asn1/asn1_par.c index 19b21e7d49..4b60c615de 100644 --- a/crypto/asn1/asn1_par.c +++ b/crypto/asn1/asn1_par.c @@ -1,5 +1,5 @@ /* - * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -38,32 +38,32 @@ static int asn1_print_info(BIO *bp, int tag, int xclass, int constructed, p = str; if ((xclass & V_ASN1_PRIVATE) == V_ASN1_PRIVATE) - sprintf(str, "priv [ %d ] ", tag); + BIO_snprintf(str, sizeof(str), "priv [ %d ] ", tag); else if ((xclass & V_ASN1_CONTEXT_SPECIFIC) == V_ASN1_CONTEXT_SPECIFIC) - sprintf(str, "cont [ %d ]", tag); + BIO_snprintf(str, sizeof(str), "cont [ %d ]", tag); else if ((xclass & V_ASN1_APPLICATION) == V_ASN1_APPLICATION) - sprintf(str, "appl [ %d ]", tag); + BIO_snprintf(str, sizeof(str), "appl [ %d ]", tag); else if (tag > 30) - sprintf(str, "", tag); + BIO_snprintf(str, sizeof(str), "", tag); else p = ASN1_tag2str(tag); if (BIO_printf(bp, fmt, p) <= 0) goto err; - return (1); + return 1; err: - return (0); + return 0; } int ASN1_parse(BIO *bp, const unsigned char *pp, long len, int indent) { - return (asn1_parse2(bp, &pp, len, 0, 0, indent, 0)); + return asn1_parse2(bp, &pp, len, 0, 0, indent, 0); } int ASN1_parse_dump(BIO *bp, const unsigned char *pp, long len, int indent, int dump) { - return (asn1_parse2(bp, &pp, len, 0, 0, indent, dump)); + return asn1_parse2(bp, &pp, len, 0, 0, indent, dump); } static int asn1_parse2(BIO *bp, const unsigned char **pp, long length, @@ -342,7 +342,7 @@ static int asn1_parse2(BIO *bp, const unsigned char **pp, long length, ASN1_OBJECT_free(o); ASN1_OCTET_STRING_free(os); *pp = p; - return (ret); + return ret; } const char *ASN1_tag2str(int tag) diff --git a/crypto/asn1/x_long.c b/crypto/asn1/x_long.c index 78f4b764c3..bf9371ef55 100644 --- a/crypto/asn1/x_long.c +++ b/crypto/asn1/x_long.c @@ -1,5 +1,5 @@ /* - * Copyright 2000-2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2000-2017 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -14,6 +14,9 @@ #if !(OPENSSL_API_COMPAT < 0x10200000L) NON_EMPTY_TRANSLATION_UNIT #else + +#define COPY_SIZE(a, b) (sizeof(a) < sizeof(b) ? sizeof(a) : sizeof(b)) + /* * Custom primitive type for long handling. This converts between an * ASN1_INTEGER and a long directly. @@ -49,13 +52,13 @@ ASN1_ITEM_end(ZLONG) static int long_new(ASN1_VALUE **pval, const ASN1_ITEM *it) { - *(long *)pval = it->size; + memcpy(pval, &it->size, COPY_SIZE(*pval, it->size)); return 1; } static void long_free(ASN1_VALUE **pval, const ASN1_ITEM *it) { - *(long *)pval = it->size; + memcpy(pval, &it->size, COPY_SIZE(*pval, it->size)); } /* @@ -90,7 +93,7 @@ static int long_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, unsigned long utmp, sign; int clen, pad, i; - ltmp = *(long *)pval; + memcpy(<mp, pval, COPY_SIZE(*pval, ltmp)); if (ltmp == it->size) return -1; /* @@ -183,13 +186,16 @@ static int long_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, ASN1err(ASN1_F_LONG_C2I, ASN1_R_INTEGER_TOO_LARGE_FOR_LONG); return 0; } - *(long*)pval = ltmp; + memcpy(pval, <mp, COPY_SIZE(*pval, ltmp)); return 1; } static int long_print(BIO *out, ASN1_VALUE **pval, const ASN1_ITEM *it, int indent, const ASN1_PCTX *pctx) { - return BIO_printf(out, "%ld\n", *(long *)pval); + long l; + + memcpy(&l, pval, COPY_SIZE(*pval, l)); + return BIO_printf(out, "%ld\n", l); } #endif