From 995d89c9f21a66602c9c3b3044ef03e779978d8b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 1 Jun 2021 11:22:56 +0200 Subject: [PATCH] Fix null pointer arithmetic in error case When mbedtls_nist_kw_wrap was called with output=NULL and out_size=0, it performed arithmetic on the null pointer before detecting that the output buffer is too small and returning an error code. This was unlikely to have consequences on real-world hardware today, but it is undefined behavior and UBSan with Clang 10 flagged it. So fix it (fix #4025). Fix a similar-looking pattern in unwrap, though I haven't verified that it's reachable there. Signed-off-by: Gilles Peskine --- library/nist_kw.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/nist_kw.c b/library/nist_kw.c index 8341ff1303..278b7e91ab 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -219,8 +219,6 @@ int mbedtls_nist_kw_wrap( mbedtls_nist_kw_context *ctx, uint64_t t = 0; unsigned char outbuff[KW_SEMIBLOCK_LENGTH * 2]; unsigned char inbuff[KW_SEMIBLOCK_LENGTH * 2]; - unsigned char *R2 = output + KW_SEMIBLOCK_LENGTH; - unsigned char *A = output; *out_len = 0; /* @@ -296,6 +294,9 @@ int mbedtls_nist_kw_wrap( mbedtls_nist_kw_context *ctx, } else { + unsigned char *R2 = output + KW_SEMIBLOCK_LENGTH; + unsigned char *A = output; + /* * Do the wrapping function W, as defined in RFC 3394 section 2.2.1 */ @@ -359,7 +360,7 @@ static int unwrap( mbedtls_nist_kw_context *ctx, uint64_t t = 0; unsigned char outbuff[KW_SEMIBLOCK_LENGTH * 2]; unsigned char inbuff[KW_SEMIBLOCK_LENGTH * 2]; - unsigned char *R = output + ( semiblocks - 2 ) * KW_SEMIBLOCK_LENGTH; + unsigned char *R = NULL; *out_len = 0; if( semiblocks < MIN_SEMIBLOCKS_COUNT ) @@ -369,6 +370,7 @@ static int unwrap( mbedtls_nist_kw_context *ctx, memcpy( A, input, KW_SEMIBLOCK_LENGTH ); memmove( output, input + KW_SEMIBLOCK_LENGTH, ( semiblocks - 1 ) * KW_SEMIBLOCK_LENGTH ); + R = output + ( semiblocks - 2 ) * KW_SEMIBLOCK_LENGTH; /* Calculate intermediate values */ for( t = s; t >= 1; t-- )