From e93a82da60f52e6fc799323b99499ee51e8c7215 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Wed, 25 Aug 2021 14:30:12 +0200 Subject: Fix instances of pointer addition with the NULL pointer ubsan found undefined pointer addtions in crypto/bio/bss_mem.c (mem_ctrl), crypto/pem/pem_lib.c (PEM_read_bio_ex), test/testutil/format_output.c (test_fail_string_common, test_fail_memory_common). Mostly a straight back-port-of: a07dc81 Additionally enable the ubsan run-checker, to prevent regressions. Reviewed-by: Tomas Mraz Reviewed-by: Nicola Tuveri (Merged from https://github.com/openssl/openssl/pull/16423) --- .github/workflows/run-checker-merge.yml | 3 +-- crypto/bio/bss_mem.c | 2 +- crypto/pem/pem_lib.c | 23 +++++++++++++---------- test/testutil/format_output.c | 12 ++++++++---- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/.github/workflows/run-checker-merge.yml b/.github/workflows/run-checker-merge.yml index 29419a2396..ff2d666b6d 100644 --- a/.github/workflows/run-checker-merge.yml +++ b/.github/workflows/run-checker-merge.yml @@ -16,8 +16,7 @@ jobs: no-engine no-shared, no-err, no-filenames, -# ubsan build is temporarily disabled, due to failures to be investigated separately -# enable-ubsan no-asm -DPEDANTIC -DOPENSSL_SMALL_FOOTPRINT -fno-sanitize=alignment, + enable-ubsan no-asm -DPEDANTIC -DOPENSSL_SMALL_FOOTPRINT -fno-sanitize=alignment, no-unit-test, enable-weak-ssl-ciphers, enable-zlib, diff --git a/crypto/bio/bss_mem.c b/crypto/bio/bss_mem.c index 7cb4a57813..14bfd00173 100644 --- a/crypto/bio/bss_mem.c +++ b/crypto/bio/bss_mem.c @@ -280,7 +280,7 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr) ret = (long)bm->length; if (ptr != NULL) { pptr = (char **)ptr; - *pptr = (char *)&(bm->data[0]); + *pptr = (char *)bm->data; } break; case BIO_C_SET_BUF_MEM: diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c index a26322119a..92dcd90a7f 100644 --- a/crypto/pem/pem_lib.c +++ b/crypto/pem/pem_lib.c @@ -899,18 +899,13 @@ err: int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, unsigned char **data, long *len_out, unsigned int flags) { - EVP_ENCODE_CTX *ctx = EVP_ENCODE_CTX_new(); + EVP_ENCODE_CTX *ctx = NULL; const BIO_METHOD *bmeth; BIO *headerB = NULL, *dataB = NULL; char *name = NULL; int len, taillen, headerlen, ret = 0; BUF_MEM * buf_mem; - if (ctx == NULL) { - PEMerr(PEM_F_PEM_READ_BIO_EX, ERR_R_MALLOC_FAILURE); - return 0; - } - *len_out = 0; *name_out = *header = NULL; *data = NULL; @@ -933,9 +928,20 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, if (!get_header_and_data(bp, &headerB, &dataB, name, flags)) goto end; - EVP_DecodeInit(ctx); BIO_get_mem_ptr(dataB, &buf_mem); len = buf_mem->length; + + /* There was no data in the PEM file */ + if (len == 0) + goto end; + + ctx = EVP_ENCODE_CTX_new(); + if (ctx == NULL) { + PEMerr(PEM_F_PEM_READ_BIO_EX, ERR_R_MALLOC_FAILURE); + goto end; + } + + EVP_DecodeInit(ctx); if (EVP_DecodeUpdate(ctx, (unsigned char*)buf_mem->data, &len, (unsigned char*)buf_mem->data, len) < 0 || EVP_DecodeFinal(ctx, (unsigned char*)&(buf_mem->data[len]), @@ -946,9 +952,6 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, len += taillen; buf_mem->length = len; - /* There was no data in the PEM file; avoid malloc(0). */ - if (len == 0) - goto end; headerlen = BIO_get_mem_data(headerB, NULL); *header = pem_malloc(headerlen + 1, flags); *data = pem_malloc(len, flags); diff --git a/test/testutil/format_output.c b/test/testutil/format_output.c index 6ee2a1d266..f42141fd8d 100644 --- a/test/testutil/format_output.c +++ b/test/testutil/format_output.c @@ -107,8 +107,10 @@ static void test_fail_string_common(const char *prefix, const char *file, if (diff && i > 0) test_printf_stderr("% 4s %s\n", "", bdiff); } - m1 += n1; - m2 += n2; + if (m1 != NULL) + m1 += n1; + if (m2 != NULL) + m2 += n2; l1 -= n1; l2 -= n2; cnt += width; @@ -495,8 +497,10 @@ static void test_fail_memory_common(const char *prefix, const char *file, if (diff && i > 0) test_printf_stderr("% 4s %s\n", "", bdiff); } - m1 += n1; - m2 += n2; + if (m1 != NULL) + m1 += n1; + if (m2 != NULL) + m2 += n2; l1 -= n1; l2 -= n2; cnt += bytes; -- cgit v1.2.3