From 4469aa38023deaccd667524d292df31874567461 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 2 Feb 2024 10:53:48 +0000 Subject: [PATCH] Reimplement BIO_dump_indent() with CBS/CBB and BIO_printf() Instead of heaps of unchecked strlcpy/strlcat/snprintf doing hard to follow gymnastics, use a byte string, a somewhat comprehensible computation of the number of bytes to dump per output line and write using checked BIO_printf() directly to the BIO. Longer strings will still overflow the terminal width of 80 and even longer strings will still overflow the return value (undefined behavior). I don't care much about the former but the latter should be fixed in a later pass. ok beck --- lib/libcrypto/bio/b_dump.c | 179 ++++++++++++++++++++++++------------- 1 file changed, 115 insertions(+), 64 deletions(-) diff --git a/lib/libcrypto/bio/b_dump.c b/lib/libcrypto/bio/b_dump.c index ef965b61d0c..09b011268e1 100644 --- a/lib/libcrypto/bio/b_dump.c +++ b/lib/libcrypto/bio/b_dump.c @@ -1,4 +1,4 @@ -/* $OpenBSD: b_dump.c,v 1.27 2024/02/01 17:04:09 tb Exp $ */ +/* $OpenBSD: b_dump.c,v 1.28 2024/02/02 10:53:48 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -56,90 +56,141 @@ * [including the GNU Public Licence.] */ -/* - * Stolen from tjh's ssl/ssl_trc.c stuff. - */ - +#include #include #include #include #include -#define TRUNCATE -#define DUMP_WIDTH 16 -#define DUMP_WIDTH_LESS_INDENT(i) (DUMP_WIDTH - ((i - (i > 6 ? 6 : i) + 3) / 4)) +#include "bytestring.h" + +#define MAX_BYTES_PER_LINE 16 + +/* + * The byte string s is dumped as lines of the following form: + * indent | byte count (4 digits) | " - " | hex dump | " " | ASCII dump + * Each byte uses 4 characters (two hex digits followed by a space and one + * ASCII character). + */ int BIO_dump_indent(BIO *bio, const char *s, int len, int indent) { - char buf[288 + 1], tmp[20], str[128 + 1]; - int i, j, rows, trc, written; - unsigned char ch; - int dump_width; - int ret = 0; + CBB cbb; + CBS cbs; + int bytes_per_line, dumped, printed, trailing, written; + int ret = -1; - trc = 0; + memset(&cbb, 0, sizeof(cbb)); -#ifdef TRUNCATE - for (; (len > 0) && ((s[len - 1] == ' ') || (s[len - 1] == '\0')); len--) - trc++; -#endif + if (len < 0) + goto err; + CBS_init(&cbs, s, len); if (indent < 0) indent = 0; if (indent > 64) indent = 64; - memset(str, ' ', indent); - str[indent] = '\0'; - - if ((dump_width = DUMP_WIDTH_LESS_INDENT(indent)) <= 0) - return -1; - rows = (len / dump_width); - if ((rows * dump_width) < len) - rows++; - for (i = 0; i < rows; i++) { - strlcpy(buf, str, sizeof buf); - snprintf(tmp, sizeof tmp, "%04x - ", i*dump_width); - strlcat(buf, tmp, sizeof buf); - for (j = 0; j < dump_width; j++) { - if (((i*dump_width) + j) >= len) { - strlcat(buf, " ", sizeof buf); - } else { - ch = ((unsigned char)*(s + i*dump_width + j)) & 0xff; - snprintf(tmp, sizeof tmp, "%02x%c", ch, - j == 7 ? '-' : ' '); - strlcat(buf, tmp, sizeof buf); - } - } - strlcat(buf, " ", sizeof buf); - for (j = 0; j < dump_width; j++) { - if (((i*dump_width) + j) >= len) - break; - ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff; - snprintf(tmp, sizeof tmp, "%c", - ((ch >= ' ') && (ch <= '~')) ? ch : '.'); - strlcat(buf, tmp, sizeof buf); - } - strlcat(buf, "\n", sizeof buf); - /* if this is the last call then update the ddt_dump thing so - * that we will move the selection point in the debug window + + /* + * Less obfuscated version of the original calculation attempting to + * ensure that the dump doesn't overshoot 80 characters per line. For + * a very long string the byte count will still make it go past that. + */ + bytes_per_line = MAX_BYTES_PER_LINE; + if (indent > 6) + bytes_per_line -= (indent - 3) / 4; + if (bytes_per_line <= 0) + goto err; + + /* Strip and count trailing spaces and NULs. */ + trailing = 0; + while (CBS_len(&cbs) > 0) { + uint8_t u8; + + if (!CBS_peek_last_u8(&cbs, &u8)) + goto err; + if (u8 != '\0' && u8 != ' ') + break; + if (!CBS_get_last_u8(&cbs, &u8)) + goto err; + trailing++; + } + + printed = 0; + dumped = 0; + while (CBS_len(&cbs) > 0) { + CBS row; + uint8_t ascii_dump[MAX_BYTES_PER_LINE]; + int missing, row_bytes; + + if ((row_bytes = CBS_len(&cbs)) > bytes_per_line) + row_bytes = bytes_per_line; + if (!CBS_get_bytes(&cbs, &row, row_bytes)) + goto err; + + /* Write out indent, byte count and initial " - ". */ + if ((written = BIO_printf(bio, "%*s%04x - ", indent, "", + dumped)) < 0) + goto err; + printed += written; + + /* + * Write out hex dump, prepare ASCII dump. */ - if ((written = BIO_write(bio, buf, strlen(buf))) < 0) - return -1; - ret += written; + if (!CBB_init_fixed(&cbb, ascii_dump, sizeof(ascii_dump))) + goto err; + while (CBS_len(&row) > 0) { + uint8_t u8; + char sep = ' '; + + if (!CBS_get_u8(&row, &u8)) + goto err; + + /* Historic behavior: print a '-' after eighth byte. */ + if (row_bytes - CBS_len(&row) == 8) + sep = '-'; + if ((written = BIO_printf(bio, "%02x%c", u8, sep)) < 0) + goto err; + printed += written; + + /* Locale-independent version of !isprint(u8). */ + if (u8 < ' ' || u8 > '~') + u8 = '.'; + if (!CBB_add_u8(&cbb, u8)) + goto err; + } + if (!CBB_finish(&cbb, NULL, NULL)) + goto err; + + /* Calculate number of bytes missing in dump of last line. */ + if ((missing = bytes_per_line - row_bytes) < 0) + goto err; + + /* Pad missing bytes, add 2 spaces and print the ASCII dump. */ + if ((written = BIO_printf(bio, "%*s%.*s\n", 3 * missing + 2, "", + row_bytes, ascii_dump)) < 0) + goto err; + printed += written; + + dumped += row_bytes; } -#ifdef TRUNCATE - if (trc > 0) { - snprintf(buf, sizeof buf, "%s%04x - \n", - str, len + trc); - if ((written = BIO_write(bio, buf, strlen(buf))) < 0) - return -1; - ret += written; + + if (trailing > 0) { + if ((written = BIO_printf(bio, "%*s%04x - \n", + indent, "", dumped + trailing)) < 0) + goto err; + printed += written; } -#endif - return (ret); + + ret = printed; + + err: + CBB_cleanup(&cbb); + + return ret; } LCRYPTO_ALIAS(BIO_dump_indent); -- 2.20.1