Fix encrypt overflow
authorMatt Caswell <matt@openssl.org>
Thu, 3 Mar 2016 23:36:23 +0000 (23:36 +0000)
committerMatt Caswell <matt@openssl.org>
Tue, 3 May 2016 07:57:06 +0000 (08:57 +0100)
An overflow can occur in the EVP_EncryptUpdate function. If an attacker is
able to supply very large amounts of input data after a previous call to
EVP_EncryptUpdate with a partial block then a length check can overflow
resulting in a heap corruption.

Following an analysis of all OpenSSL internal usage of the
EVP_EncryptUpdate function all usage is one of two forms.

The first form is like this:
EVP_EncryptInit()
EVP_EncryptUpdate()

i.e. where the EVP_EncryptUpdate() call is known to be the first called
function after an EVP_EncryptInit(), and therefore that specific call
must be safe.

The second form is where the length passed to EVP_EncryptUpdate() can be
seen from the code to be some small value and therefore there is no
possibility of an overflow.

Since all instances are one of these two forms, I believe that there can
be no overflows in internal code due to this problem.

It should be noted that EVP_DecryptUpdate() can call EVP_EncryptUpdate()
in certain code paths. Also EVP_CipherUpdate() is a synonym for
EVP_EncryptUpdate(). Therefore I have checked all instances of these
calls too, and came to the same conclusion, i.e. there are no instances
in internal usage where an overflow could occur.

This could still represent a security issue for end user code that calls
this function directly.

CVE-2016-2106

Issue reported by Guido Vranken.

Reviewed-by: Tim Hudson <tjh@openssl.org>
crypto/evp/evp_enc.c

index d7cc1f453c4114db738cae9f37561e9f807a6255..e02e801949c0eb883ad8def26f63875ec6dda2ed 100644 (file)
@@ -332,7 +332,7 @@ int EVP_EncryptUpdate(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl,
     bl = ctx->cipher->block_size;
     OPENSSL_assert(bl <= (int)sizeof(ctx->buf));
     if (i != 0) {
     bl = ctx->cipher->block_size;
     OPENSSL_assert(bl <= (int)sizeof(ctx->buf));
     if (i != 0) {
-        if (i + inl < bl) {
+        if (bl - i > inl) {
             memcpy(&(ctx->buf[i]), in, inl);
             ctx->buf_len += inl;
             *outl = 0;
             memcpy(&(ctx->buf[i]), in, inl);
             ctx->buf_len += inl;
             *outl = 0;