diff options
author | David Benjamin <davidben@chromium.org> | 2014-08-04 23:06:28 +0400 |
---|---|---|
committer | Adam Langley <agl@google.com> | 2014-08-05 01:11:21 +0400 |
commit | aa4efe76694725ad242f763d004d538ccbba6533 (patch) | |
tree | 2db5f66c13ad7e177f18539ccf5b0a90c2796f85 /crypto/bio | |
parent | a59fbb0eddf6eb129ab994a5331bf77ab8721eb0 (diff) |
Fix BIO_printf crash on Mac.
A single va_list may not be used twice. Nothing calls BIO_vprintf and it just
(v)snprintfs into a buffer anyway, so remove it. If it's actually needed, we
can fiddle with va_copy and the lack of it in C89 later, but anything that
actually cares can just assemble the output externally.
Add a test in bio_test.c.
BUG=399546
Change-Id: Ia40a68b31cb5984d817e9c55351f49d9d6c964c1
Reviewed-on: https://boringssl-review.googlesource.com/1391
Reviewed-by: Adam Langley <agl@google.com>
Diffstat (limited to 'crypto/bio')
-rw-r--r-- | crypto/bio/bio_test.c | 55 | ||||
-rw-r--r-- | crypto/bio/printf.c | 28 |
2 files changed, 65 insertions, 18 deletions
diff --git a/crypto/bio/bio_test.c b/crypto/bio/bio_test.c index 8b3c129d..a8e46abd 100644 --- a/crypto/bio/bio_test.c +++ b/crypto/bio/bio_test.c @@ -17,6 +17,7 @@ #include <arpa/inet.h> #include <fcntl.h> #include <netinet/in.h> +#include <string.h> #include <sys/socket.h> #include <unistd.h> @@ -93,6 +94,56 @@ static int test_socket_connect() { return 1; } +static int test_printf() { + /* Test a short output, a very long one, and various sizes around + * 256 (the size of the buffer) to ensure edge cases are correct. */ + static const size_t kLengths[] = { 5, 250, 251, 252, 253, 254, 1023 }; + BIO *bio; + char string[1024]; + int ret; + const uint8_t *contents; + size_t len; + + bio = BIO_new(BIO_s_mem()); + if (!bio) { + fprintf(stderr, "BIO_new failed\n"); + return 0; + } + + for (size_t i = 0; i < sizeof(kLengths) / sizeof(kLengths[0]); i++) { + if (kLengths[i] >= sizeof(string)) { + fprintf(stderr, "Bad test string length\n"); + return 0; + } + memset(string, 'a', sizeof(string)); + string[kLengths[i]] = '\0'; + + ret = BIO_printf(bio, "test %s", string); + if (ret != 5 + kLengths[i]) { + fprintf(stderr, "BIO_printf failed\n"); + return 0; + } + if (!BIO_mem_contents(bio, &contents, &len)) { + fprintf(stderr, "BIO_mem_contents failed\n"); + return 0; + } + if (len != 5 + kLengths[i] || + strncmp((const char *)contents, "test ", 5) != 0 || + strncmp((const char *)contents + 5, string, kLengths[i]) != 0) { + fprintf(stderr, "Contents did not match: %.*s\n", (int)len, contents); + return 0; + } + + if (!BIO_reset(bio)) { + fprintf(stderr, "BIO_reset failed\n"); + return 0; + } + } + + BIO_free(bio); + return 1; +} + int main() { ERR_load_crypto_strings(); @@ -100,6 +151,10 @@ int main() { return 1; } + if (!test_printf()) { + return 1; + } + printf("PASS\n"); return 0; } diff --git a/crypto/bio/printf.c b/crypto/bio/printf.c index 7f7106f4..daebe74c 100644 --- a/crypto/bio/printf.c +++ b/crypto/bio/printf.c @@ -66,38 +66,30 @@ #include <openssl/mem.h> - int BIO_printf(BIO *bio, const char *format, ...) { va_list args; - int ret; - - va_start(args, format); - - ret = BIO_vprintf(bio, format, args); - - va_end(args); - return ret; -} - -int BIO_vprintf(BIO *bio, const char *format, va_list args) { char buf[256], *out, out_malloced = 0; int out_len, ret; - /* Note: this is assuming that va_list is ok to copy as POD. If the system - * defines it with a pointer to mutable state then passing it twice to - * vsnprintf will not work. */ + va_start(args, format); out_len = vsnprintf(buf, sizeof(buf), format, args); + va_end(args); + if (out_len >= sizeof(buf)) { const int requested_len = out_len; - /* The output was truncated. */ - out = OPENSSL_malloc(requested_len); + /* The output was truncated. Note that vsnprintf's return value + * does not include a trailing NUL, but the buffer must be sized + * for it. */ + out = OPENSSL_malloc(requested_len + 1); out_malloced = 1; if (out == NULL) { /* Unclear what can be done in this situation. OpenSSL has historically * crashed and that seems better than producing the wrong output. */ abort(); } - out_len = vsnprintf(out, requested_len, format, args); + va_start(args, format); + out_len = vsnprintf(out, requested_len + 1, format, args); + va_end(args); assert(out_len == requested_len); } else { out = buf; |