Don't overflow the output length in EVP_CipherUpdate calls.
CVE-2021-23840
(Imported from upstream's 6a51b9e1d0cf0bf8515f7201b68fb0a3482b3dc1.)
This differs slightly from upstream's version:
- EVP_R_OUTPUT_WOULD_OVERFLOW didn't seem necessary when ERR_R_OVERFLOW
already exists. (Also since we use CIPHER_R_*, it wouldn't have helped
with compatibility anyway. Though there's probably something to be
said for us folding CIPHER_R_* back into EVP_R_*.)
- For simplicity, just check in_len + bl at the top, rather than trying
to predict the exact number of bytes written.
Update-Note: Passing extremely large input lengths into EVP_CipherUpdate
will now fail. Use EVP_AEAD instead, which is size_t-based and has more
explicit output bounds.
Change-Id: I31835c89dcdecb6b112828f57deb798dc7187db5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45685
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/cipher/cipher.c b/crypto/fipsmodule/cipher/cipher.c
index c50c6c5..1522379 100644
--- a/crypto/fipsmodule/cipher/cipher.c
+++ b/crypto/fipsmodule/cipher/cipher.c
@@ -57,6 +57,7 @@
#include <openssl/cipher.h>
#include <assert.h>
+#include <limits.h>
#include <string.h>
#include <openssl/err.h>
@@ -240,14 +241,20 @@
int EVP_EncryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
const uint8_t *in, int in_len) {
- int i, j, bl;
+ // Ciphers that use blocks may write up to |bl| extra bytes. Ensure the output
+ // does not overflow |*out_len|.
+ int bl = ctx->cipher->block_size;
+ if (bl > 1 && in_len > INT_MAX - bl) {
+ OPENSSL_PUT_ERROR(CIPHER, ERR_R_OVERFLOW);
+ return 0;
+ }
if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) {
- i = ctx->cipher->cipher(ctx, out, in, in_len);
- if (i < 0) {
+ int ret = ctx->cipher->cipher(ctx, out, in, in_len);
+ if (ret < 0) {
return 0;
} else {
- *out_len = i;
+ *out_len = ret;
}
return 1;
}
@@ -267,8 +274,7 @@
}
}
- i = ctx->buf_len;
- bl = ctx->cipher->block_size;
+ int i = ctx->buf_len;
assert(bl <= (int)sizeof(ctx->buf));
if (i != 0) {
if (bl - i > in_len) {
@@ -277,7 +283,7 @@
*out_len = 0;
return 1;
} else {
- j = bl - i;
+ int j = bl - i;
OPENSSL_memcpy(&ctx->buf[i], in, j);
if (!ctx->cipher->cipher(ctx, out, ctx->buf, bl)) {
return 0;
@@ -353,8 +359,13 @@
int EVP_DecryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
const uint8_t *in, int in_len) {
- int fix_len;
- unsigned int b;
+ // Ciphers that use blocks may write up to |bl| extra bytes. Ensure the output
+ // does not overflow |*out_len|.
+ unsigned int b = ctx->cipher->block_size;
+ if (b > 1 && in_len > INT_MAX - (int)b) {
+ OPENSSL_PUT_ERROR(CIPHER, ERR_R_OVERFLOW);
+ return 0;
+ }
if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) {
int r = ctx->cipher->cipher(ctx, out, in, in_len);
@@ -376,15 +387,12 @@
return EVP_EncryptUpdate(ctx, out, out_len, in, in_len);
}
- b = ctx->cipher->block_size;
assert(b <= sizeof(ctx->final));
-
+ int fix_len = 0;
if (ctx->final_used) {
OPENSSL_memcpy(out, ctx->final, b);
out += b;
fix_len = 1;
- } else {
- fix_len = 0;
}
if (!EVP_EncryptUpdate(ctx, out, out_len, in, in_len)) {
diff --git a/include/openssl/cipher.h b/include/openssl/cipher.h
index c6bec48..3feadea 100644
--- a/include/openssl/cipher.h
+++ b/include/openssl/cipher.h
@@ -558,6 +558,10 @@
// block_mask contains |cipher->block_size| minus one. (The block size
// assumed to be a power of two.)
+ //
+ // TODO(davidben): This is redundant with |cipher->block_size| and constant
+ // for the whole |EVP_CIPHER|. Move it there, or possibly even remove it and
+ // do the subtraction on demand.
int block_mask;
uint8_t final[EVP_MAX_BLOCK_LENGTH]; // possible final block
