From 86ba26c80a49aee3c588d286d91eb3843529f7e2 Mon Sep 17 00:00:00 2001 From: Pauli Date: Fri, 7 Jul 2017 10:17:59 +1000 Subject: [PATCH] Address potential buffer overflows. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3878) --- crypto/bn/bn_print.c | 27 ++++++++++--------- crypto/mem_dbg.c | 62 ++++++++++++++++++++++++++++++++------------ crypto/pem/pem_lib.c | 37 ++++++++++++++------------ 3 files changed, 80 insertions(+), 46 deletions(-) diff --git a/crypto/bn/bn_print.c b/crypto/bn/bn_print.c index 956b2d520f..9f849978d8 100644 --- a/crypto/bn/bn_print.c +++ b/crypto/bn/bn_print.c @@ -52,7 +52,7 @@ char *BN_bn2hex(const BIGNUM *a) /* Must 'OPENSSL_free' the returned data */ char *BN_bn2dec(const BIGNUM *a) { - int i = 0, num, ok = 0; + int i = 0, num, ok = 0, n, tbytes; char *buf = NULL; char *p; BIGNUM *t = NULL; @@ -67,9 +67,10 @@ char *BN_bn2dec(const BIGNUM *a) */ i = BN_num_bits(a) * 3; num = (i / 10 + i / 1000 + 1) + 1; + tbytes = num + 3; /* negative and terminator and one spare? */ bn_data_num = num / BN_DEC_NUM + 1; bn_data = OPENSSL_malloc(bn_data_num * sizeof(BN_ULONG)); - buf = OPENSSL_malloc(num + 3); + buf = OPENSSL_malloc(tbytes); if ((buf == NULL) || (bn_data == NULL)) { BNerr(BN_F_BN_BN2DEC, ERR_R_MALLOC_FAILURE); goto err; @@ -100,14 +101,16 @@ char *BN_bn2dec(const BIGNUM *a) * the last one needs truncation. The blocks need to be reversed in * order. */ - sprintf(p, BN_DEC_FMT1, *lp); - while (*p) - p++; + n = BIO_snprintf(p, tbytes - (size_t)(p - buf), BN_DEC_FMT1, *lp); + if (n < 0) + goto err; + p += n; while (lp != bn_data) { lp--; - sprintf(p, BN_DEC_FMT2, *lp); - while (*p) - p++; + n = BIO_snprintf(p, tbytes - (size_t)(p - buf), BN_DEC_FMT2, *lp); + if (n < 0) + goto err; + p += n; } } ok = 1; @@ -331,11 +334,11 @@ char *BN_options(void) if (!init) { init++; #ifdef BN_LLONG - sprintf(data, "bn(%d,%d)", - (int)sizeof(BN_ULLONG) * 8, (int)sizeof(BN_ULONG) * 8); + BIO_snprintf(data, sizeof(data), "bn(%zu,%zu)", + sizeof(BN_ULLONG) * 8, sizeof(BN_ULONG) * 8); #else - sprintf(data, "bn(%d,%d)", - (int)sizeof(BN_ULONG) * 8, (int)sizeof(BN_ULONG) * 8); + BIO_snprintf(data, sizeof(data), "bn(%zu,%zu)", + sizeof(BN_ULONG) * 8, sizeof(BN_ULONG) * 8); #endif } return data; diff --git a/crypto/mem_dbg.c b/crypto/mem_dbg.c index 1ab52a86e8..70b5e62ab5 100644 --- a/crypto/mem_dbg.c +++ b/crypto/mem_dbg.c @@ -453,8 +453,9 @@ static void print_leak(const MEM *m, MEM_LEAK *l) { char buf[1024]; char *bufp = buf; + size_t len = sizeof(buf), ami_cnt; APP_INFO *amip; - int ami_cnt; + int n; struct tm *lcl = NULL; /* * Convert between CRYPTO_THREAD_ID (which could be anything at all) and @@ -468,21 +469,37 @@ static void print_leak(const MEM *m, MEM_LEAK *l) CRYPTO_THREAD_ID ti; lcl = localtime(&m->time); - sprintf(bufp, "[%02d:%02d:%02d] ", lcl->tm_hour, lcl->tm_min, lcl->tm_sec); - bufp += strlen(bufp); + n = BIO_snprintf(bufp, len, "[%02d:%02d:%02d] ", + lcl->tm_hour, lcl->tm_min, lcl->tm_sec); + if (n <= 0) { + bufp[0] = '\0'; + return; + } + bufp += n; + len -= n; - sprintf(bufp, "%5lu file=%s, line=%d, ", m->order, m->file, m->line); - bufp += strlen(bufp); + n = BIO_snprintf(bufp, len, "%5lu file=%s, line=%d, ", + m->order, m->file, m->line); + if (n <= 0) + return; + bufp += n; + len -= n; tid.ltid = 0; tid.tid = m->threadid; - sprintf(bufp, "thread=%lu, ", tid.ltid); - bufp += strlen(bufp); + n = BIO_snprintf(bufp, len, "thread=%lu, ", tid.ltid); + if (n <= 0) + return; + bufp += n; + len -= n; - sprintf(bufp, "number=%d, address=%p\n", m->num, m->addr); - bufp += strlen(bufp); + n = BIO_snprintf(bufp, len, "number=%d, address=%p\n", m->num, m->addr); + if (n <= 0) + return; + bufp += n; + len -= n; - l->print_cb(buf, strlen(buf), l->print_cb_arg); + l->print_cb(buf, (size_t)(bufp - buf), l->print_cb_arg); l->chunks++; l->bytes += m->num; @@ -498,23 +515,34 @@ static void print_leak(const MEM *m, MEM_LEAK *l) int info_len; ami_cnt++; + if (ami_cnt >= sizeof(buf) - 1) + break; memset(buf, '>', ami_cnt); + buf[ami_cnt] = '\0'; tid.ltid = 0; tid.tid = amip->threadid; - sprintf(buf + ami_cnt, " thread=%lu, file=%s, line=%d, info=\"", - tid.ltid, amip->file, amip->line); - buf_len = strlen(buf); + n = BIO_snprintf(buf + ami_cnt, sizeof(buf) - ami_cnt, + " thread=%lu, file=%s, line=%d, info=\"", + tid.ltid, amip->file, amip->line); + if (n <= 0) + break; + buf_len = ami_cnt + n; info_len = strlen(amip->info); if (128 - buf_len - 3 < info_len) { memcpy(buf + buf_len, amip->info, 128 - buf_len - 3); buf_len = 128 - 3; } else { - strcpy(buf + buf_len, amip->info); - buf_len = strlen(buf); + n = BIO_snprintf(buf + buf_len, sizeof(buf) - buf_len, "%s", + amip->info); + if (n < 0) + break; + buf_len += n; } - sprintf(buf + buf_len, "\"\n"); + n = BIO_snprintf(buf + buf_len, sizeof(buf) - buf_len, "\"\n"); + if (n <= 0) + break; - l->print_cb(buf, strlen(buf), l->print_cb_arg); + l->print_cb(buf, buf_len + n, l->print_cb_arg); amip = amip->next; } diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c index 7e5e3747b0..aacdad9e63 100644 --- a/crypto/pem/pem_lib.c +++ b/crypto/pem/pem_lib.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 @@ -71,6 +71,7 @@ int PEM_def_callback(char *buf, int num, int w, void *key) void PEM_proc_type(char *buf, int type) { const char *str; + char *p = buf + strlen(buf); if (type == PEM_TYPE_ENCRYPTED) str = "ENCRYPTED"; @@ -81,27 +82,29 @@ void PEM_proc_type(char *buf, int type) else str = "BAD-TYPE"; - strcat(buf, "Proc-Type: 4,"); - strcat(buf, str); - strcat(buf, "\n"); + BIO_snprintf(p, PEM_BUFSIZE - (size_t)(p - buf), "Proc-Type: 4,%s\n", str); } void PEM_dek_info(char *buf, const char *type, int len, char *str) { - static const unsigned char map[17] = "0123456789ABCDEF"; long i; - int j; - - strcat(buf, "DEK-Info: "); - strcat(buf, type); - strcat(buf, ","); - j = strlen(buf); - for (i = 0; i < len; i++) { - buf[j + i * 2] = map[(str[i] >> 4) & 0x0f]; - buf[j + i * 2 + 1] = map[(str[i]) & 0x0f]; - } - buf[j + i * 2] = '\n'; - buf[j + i * 2 + 1] = '\0'; + char *p = buf + strlen(buf); + int j = PEM_BUFSIZE - (size_t)(p - buf), n; + + n = BIO_snprintf(p, j, "DEK-Info: %s,", type); + if (n > 0) { + j -= n; + p += n; + for (i = 0; i < len; i++) { + n = BIO_snprintf(p, j, "%02X", 0xff & str[i]); + if (n <= 0) + return; + j -= n; + p += n; + } + if (j > 1) + strcpy(p, "\n"); + } } #ifndef OPENSSL_NO_STDIO -- GitLab