From e6abba3ad6107d35a6e8b01a1a145902edf0062d Mon Sep 17 00:00:00 2001 From: Richard Godbee Date: Fri, 13 Mar 2015 21:23:21 -0700 Subject: [PATCH] CRYPTO_128_unwrap(): Fix refactoring damage MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit crypto/modes/wrap128.c was heavily refactored to support AES Key Wrap with Padding, and four bugs were introduced into CRYPTO_128_unwrap() at that time: - crypto_128_unwrap_raw()'s return value ('ret') is checked incorrectly, and the function immediately returns 'ret' in (almost) all cases. This makes the IV checking code later in the function unreachable, but callers think the IV check succeeded since CRYPTO_128_unwrap()'s return value is non-zero. FIX: Return 0 (error) if crypto_128_unwrap_raw() returned 0 (error). - crypto_128_unwrap_raw() writes the IV to the 'got_iv' buffer, not to the first 8 bytes of the output buffer ('out') as the IV checking code expects. This makes the IV check fail. FIX: Compare 'iv' to 'got_iv', not 'out'. - The data written to the output buffer ('out') is "cleansed" if the IV check fails, but the code passes OPENSSL_cleanse() the input buffer length ('inlen') instead of the number of bytes that crypto_128_unwrap_raw() wrote to the output buffer ('ret'). This means that OPENSSL_cleanse() could potentially write past the end of 'out'. FIX: Change 'inlen' to 'ret' in the OPENSSL_cleanse() call. - CRYPTO_128_unwrap() is returning the length of the input buffer ('inlen') instead of the number of bytes written to the output buffer ('ret'). This could cause the caller to read past the end of 'out'. FIX: Return 'ret' instead of 'inlen' at the end of the function. PR#3749 Reviewed-by: Stephen Henson Reviewed-by: Emilia Käsper --- crypto/modes/wrap128.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crypto/modes/wrap128.c b/crypto/modes/wrap128.c index e1191e496e..fe33a986be 100644 --- a/crypto/modes/wrap128.c +++ b/crypto/modes/wrap128.c @@ -201,16 +201,16 @@ size_t CRYPTO_128_unwrap(void *key, const unsigned char *iv, unsigned char got_iv[8]; ret = crypto_128_unwrap_raw(key, got_iv, out, in, inlen, block); - if (ret != inlen) - return ret; + if (ret == 0) + return 0; if (!iv) iv = default_iv; - if (CRYPTO_memcmp(out, iv, 8)) { - OPENSSL_cleanse(out, inlen); + if (CRYPTO_memcmp(got_iv, iv, 8)) { + OPENSSL_cleanse(out, ret); return 0; } - return inlen; + return ret; } /** Wrapping according to RFC 5649 section 4.1. -- 2.34.1