From 59e539e6429d9c3b5c4db07569f09ec6acc5a7e9 Mon Sep 17 00:00:00 2001 From: Pauli Date: Thu, 6 Jul 2017 14:11:27 +1000 Subject: [PATCH] BIO range checking. Add length limits to avoid problems with sprintf, strcpy and strcat. This replaces recently removed code but also guards some previously missing function calls (for DOS & Windows). Reworked the BIO_dump_indent_cb code to reduce temporary storage. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/3870) --- crypto/bio/b_dump.c | 74 ++++++++++++++++++++++++------------------- crypto/bio/bio_cb.c | 51 ++++++++++++++--------------- crypto/bio/bss_file.c | 46 +++++++++++++-------------- 3 files changed, 90 insertions(+), 81 deletions(-) diff --git a/crypto/bio/b_dump.c b/crypto/bio/b_dump.c index 491b973369..f5391406e8 100644 --- a/crypto/bio/b_dump.c +++ b/crypto/bio/b_dump.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 @@ -16,7 +16,9 @@ #define TRUNCATE #define DUMP_WIDTH 16 -#define DUMP_WIDTH_LESS_INDENT(i) (DUMP_WIDTH-((i-(i>6?6:i)+3)/4)) +#define DUMP_WIDTH_LESS_INDENT(i) (DUMP_WIDTH - ((i - (i > 6 ? 6 : i) + 3) / 4)) + +#define SPACE(buf, pos, n) (sizeof(buf) - (pos) > (n)) int BIO_dump_cb(int (*cb) (const void *data, size_t len, void *u), void *u, const char *s, int len) @@ -28,8 +30,8 @@ int BIO_dump_indent_cb(int (*cb) (const void *data, size_t len, void *u), void *u, const char *s, int len, int indent) { int ret = 0; - char buf[288 + 1], tmp[20], str[128 + 1]; - int i, j, rows, trc; + char buf[288 + 1]; + int i, j, rows, trc, n; unsigned char ch; int dump_width; @@ -42,59 +44,65 @@ int BIO_dump_indent_cb(int (*cb) (const void *data, size_t len, void *u), if (indent < 0) indent = 0; - if (indent) { - if (indent > 128) - indent = 128; - memset(str, ' ', indent); - } - str[indent] = '\0'; + else if (indent > 128) + indent = 128; dump_width = DUMP_WIDTH_LESS_INDENT(indent); - rows = (len / dump_width); + rows = len / dump_width; if ((rows * dump_width) < len) rows++; for (i = 0; i < rows; i++) { - strcpy(buf, str); - sprintf(tmp, "%04x - ", i * dump_width); - strcat(buf, tmp); + n = BIO_snprintf(buf, sizeof(buf), "%*s%04x - ", indent, "", + i * dump_width); for (j = 0; j < dump_width; j++) { - if (((i * dump_width) + j) >= len) { - strcat(buf, " "); - } else { - ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff; - sprintf(tmp, "%02x%c", ch, j == 7 ? '-' : ' '); - strcat(buf, tmp); + if (SPACE(buf, n, 3)) { + if (((i * dump_width) + j) >= len) { + strcpy(buf + n, " "); + } else { + ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff; + BIO_snprintf(buf + n, 4, "%02x%c", ch, + j == 7 ? '-' : ' '); + } + n += 3; } } - strcat(buf, " "); + if (SPACE(buf, n, 2)) { + strcpy(buf + n, " "); + n += 2; + } for (j = 0; j < dump_width; j++) { if (((i * dump_width) + j) >= len) break; - ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff; + if (SPACE(buf, n, 1)) { + ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff; #ifndef CHARSET_EBCDIC - sprintf(tmp, "%c", ((ch >= ' ') && (ch <= '~')) ? ch : '.'); + buf[n++] = ((ch >= ' ') && (ch <= '~')) ? ch : '.'; #else - sprintf(tmp, "%c", - ((ch >= os_toascii[' ']) && (ch <= os_toascii['~'])) - ? os_toebcdic[ch] - : '.'); + buf[n++] = ((ch >= os_toascii[' ']) && (ch <= os_toascii['~'])) + ? os_toebcdic[ch] + : '.'; #endif - strcat(buf, tmp); + buf[n] = '\0'; + } + } + if (SPACE(buf, n, 1)) { + buf[n++] = '\n'; + buf[n] = '\0'; } - strcat(buf, "\n"); /* * if this is the last call then update the ddt_dump thing so that we * will move the selection point in the debug window */ - ret += cb((void *)buf, strlen(buf), u); + ret += cb((void *)buf, n, u); } #ifdef TRUNCATE if (trc > 0) { - sprintf(buf, "%s%04x - \n", str, len + trc); - ret += cb((void *)buf, strlen(buf), u); + n = BIO_snprintf(buf, sizeof(buf), "%*s%04x - \n", + indent, "", len + trc); + ret += cb((void *)buf, n, u); } #endif - return (ret); + return ret; } #ifndef OPENSSL_NO_STDIO diff --git a/crypto/bio/bio_cb.c b/crypto/bio/bio_cb.c index 13368e82ee..1154c233af 100644 --- a/crypto/bio/bio_cb.c +++ b/crypto/bio/bio_cb.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,68 +21,69 @@ long BIO_debug_callback(BIO *bio, int cmd, const char *argp, char buf[256]; char *p; long r = 1; - int len; + int len, left; if (BIO_CB_RETURN & cmd) r = ret; - len = sprintf(buf, "BIO[%p]: ", (void *)bio); + len = BIO_snprintf(buf, sizeof(buf), "BIO[%p]: ", (void *)bio); /* Ignore errors and continue printing the other information. */ if (len < 0) len = 0; p = buf + len; + left = sizeof(buf) - len; switch (cmd) { case BIO_CB_FREE: - sprintf(p, "Free - %s\n", bio->method->name); + BIO_snprintf(p, left, "Free - %s\n", bio->method->name); break; case BIO_CB_READ: if (bio->method->type & BIO_TYPE_DESCRIPTOR) - sprintf(p, "read(%d,%lu) - %s fd=%d\n", - bio->num, (unsigned long)argi, - bio->method->name, bio->num); + BIO_snprintf(p, left, "read(%d,%lu) - %s fd=%d\n", + bio->num, (unsigned long)argi, + bio->method->name, bio->num); else - sprintf(p, "read(%d,%lu) - %s\n", + BIO_snprintf(p, left, "read(%d,%lu) - %s\n", bio->num, (unsigned long)argi, bio->method->name); break; case BIO_CB_WRITE: if (bio->method->type & BIO_TYPE_DESCRIPTOR) - sprintf(p, "write(%d,%lu) - %s fd=%d\n", - bio->num, (unsigned long)argi, - bio->method->name, bio->num); + BIO_snprintf(p, left, "write(%d,%lu) - %s fd=%d\n", + bio->num, (unsigned long)argi, + bio->method->name, bio->num); else - sprintf(p, "write(%d,%lu) - %s\n", - bio->num, (unsigned long)argi, bio->method->name); + BIO_snprintf(p, left, "write(%d,%lu) - %s\n", + bio->num, (unsigned long)argi, bio->method->name); break; case BIO_CB_PUTS: - sprintf(p, "puts() - %s\n", bio->method->name); + BIO_snprintf(p, left, "puts() - %s\n", bio->method->name); break; case BIO_CB_GETS: - sprintf(p, "gets(%lu) - %s\n", (unsigned long)argi, - bio->method->name); + BIO_snprintf(p, left, "gets(%lu) - %s\n", (unsigned long)argi, + bio->method->name); break; case BIO_CB_CTRL: - sprintf(p, "ctrl(%lu) - %s\n", (unsigned long)argi, - bio->method->name); + BIO_snprintf(p, left, "ctrl(%lu) - %s\n", (unsigned long)argi, + bio->method->name); break; case BIO_CB_RETURN | BIO_CB_READ: - sprintf(p, "read return %ld\n", ret); + BIO_snprintf(p, left, "read return %ld\n", ret); break; case BIO_CB_RETURN | BIO_CB_WRITE: - sprintf(p, "write return %ld\n", ret); + BIO_snprintf(p, left, "write return %ld\n", ret); break; case BIO_CB_RETURN | BIO_CB_GETS: - sprintf(p, "gets return %ld\n", ret); + BIO_snprintf(p, left, "gets return %ld\n", ret); break; case BIO_CB_RETURN | BIO_CB_PUTS: - sprintf(p, "puts return %ld\n", ret); + BIO_snprintf(p, left, "puts return %ld\n", ret); break; case BIO_CB_RETURN | BIO_CB_CTRL: - sprintf(p, "ctrl return %ld\n", ret); + BIO_snprintf(p, left, "ctrl return %ld\n", ret); break; default: - sprintf(p, "bio callback - unknown type (%d)\n", cmd); + BIO_snprintf(p, left, "bio callback - unknown type (%d)\n", cmd); break; } @@ -93,5 +94,5 @@ long BIO_debug_callback(BIO *bio, int cmd, const char *argp, else fputs(buf, stderr); #endif - return (r); + return r; } diff --git a/crypto/bio/bss_file.c b/crypto/bio/bss_file.c index 49d8f09bc5..e7bbc317a3 100644 --- a/crypto/bio/bss_file.c +++ b/crypto/bio/bss_file.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 @@ -85,17 +85,17 @@ BIO *BIO_new_file(const char *filename, const char *mode) BIOerr(BIO_F_BIO_NEW_FILE, BIO_R_NO_SUCH_FILE); else BIOerr(BIO_F_BIO_NEW_FILE, ERR_R_SYS_LIB); - return (NULL); + return NULL; } if ((ret = BIO_new(BIO_s_file())) == NULL) { fclose(file); - return (NULL); + return NULL; } BIO_clear_flags(ret, BIO_FLAGS_UPLINK); /* we did fopen -> we disengage * UPLINK */ BIO_set_fp(ret, file, fp_flags); - return (ret); + return ret; } BIO *BIO_new_fp(FILE *stream, int close_flag) @@ -103,17 +103,17 @@ BIO *BIO_new_fp(FILE *stream, int close_flag) BIO *ret; if ((ret = BIO_new(BIO_s_file())) == NULL) - return (NULL); + return NULL; /* redundant flag, left for documentation purposes */ BIO_set_flags(ret, BIO_FLAGS_UPLINK); BIO_set_fp(ret, stream, close_flag); - return (ret); + return ret; } const BIO_METHOD *BIO_s_file(void) { - return (&methods_filep); + return &methods_filep; } static int file_new(BIO *bi) @@ -122,13 +122,13 @@ static int file_new(BIO *bi) bi->num = 0; bi->ptr = NULL; bi->flags = BIO_FLAGS_UPLINK; /* default to UPLINK */ - return (1); + return 1; } static int file_free(BIO *a) { if (a == NULL) - return (0); + return 0; if (a->shutdown) { if ((a->init) && (a->ptr != NULL)) { if (a->flags & BIO_FLAGS_UPLINK) @@ -140,7 +140,7 @@ static int file_free(BIO *a) } a->init = 0; } - return (1); + return 1; } static int file_read(BIO *b, char *out, int outl) @@ -160,7 +160,7 @@ static int file_read(BIO *b, char *out, int outl) ret = -1; } } - return (ret); + return ret; } static int file_write(BIO *b, const char *in, int inl) @@ -181,7 +181,7 @@ static int file_write(BIO *b, const char *in, int inl) * implementations (VMS) */ } - return (ret); + return ret; } static long file_ctrl(BIO *b, int cmd, long num, void *ptr) @@ -271,15 +271,15 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) b->shutdown = (int)num & BIO_CLOSE; if (num & BIO_FP_APPEND) { if (num & BIO_FP_READ) - strcpy(p, "a+"); + OPENSSL_strlcpy(p, "a+", sizeof(p)); else - strcpy(p, "a"); + OPENSSL_strlcpy(p, "a", sizeof(p)); } else if ((num & BIO_FP_READ) && (num & BIO_FP_WRITE)) - strcpy(p, "r+"); + OPENSSL_strlcpy(p, "r+", sizeof(p)); else if (num & BIO_FP_WRITE) - strcpy(p, "w"); + OPENSSL_strlcpy(p, "w", sizeof(p)); else if (num & BIO_FP_READ) - strcpy(p, "r"); + OPENSSL_strlcpy(p, "r", sizeof(p)); else { BIOerr(BIO_F_FILE_CTRL, BIO_R_BAD_FOPEN_MODE); ret = 0; @@ -287,9 +287,9 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) } # if defined(OPENSSL_SYS_MSDOS) || defined(OPENSSL_SYS_WINDOWS) || defined(OPENSSL_SYS_WIN32_CYGWIN) if (!(num & BIO_FP_TEXT)) - strcat(p, "b"); + OPENSSL_strlcat(p, "b", sizeof(p)); else - strcat(p, "t"); + OPENSSL_strlcat(p, "t", sizeof(p)); # endif fp = openssl_fopen(ptr, p); if (fp == NULL) { @@ -339,7 +339,7 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) ret = 0; break; } - return (ret); + return ret; } static int file_gets(BIO *bp, char *buf, int size) @@ -357,7 +357,7 @@ static int file_gets(BIO *bp, char *buf, int size) if (buf[0] != '\0') ret = strlen(buf); err: - return (ret); + return ret; } static int file_puts(BIO *bp, const char *str) @@ -366,7 +366,7 @@ static int file_puts(BIO *bp, const char *str) n = strlen(str); ret = file_write(bp, str, n); - return (ret); + return ret; } #else @@ -419,7 +419,7 @@ static const BIO_METHOD methods_filep = { const BIO_METHOD *BIO_s_file(void) { - return (&methods_filep); + return &methods_filep; } BIO *BIO_new_file(const char *filename, const char *mode) -- 2.25.1