From: tb <tb@openbsd.org>
Date: Sat, 18 Nov 2023 09:37:15 +0000 (+0000)
Subject: Check for negative IV length
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=0a5044de2e5201161f19ce9796a2921769c4cbb1;p=openbsd

Check for negative IV length

A recent change in EVP_CIPHER_CTX_iv_length() made it possible in principle
that this function returns -1. This can only happen for an incorrectly set
up EVP_CIPHER. Still it is better form to check for negative lengths before
stuffing it into a memcpy().

It would probably be desirable to cap the iv_length to something large
enough. This can be done another time.

ok beck
---

diff --git a/lib/libcrypto/evp/e_aes.c b/lib/libcrypto/evp/e_aes.c
index 3d357f01197..eb7f5202824 100644
--- a/lib/libcrypto/evp/e_aes.c
+++ b/lib/libcrypto/evp/e_aes.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: e_aes.c,v 1.54 2023/09/28 11:29:10 tb Exp $ */
+/* $OpenBSD: e_aes.c,v 1.55 2023/11/18 09:37:15 tb Exp $ */
 /* ====================================================================
  * Copyright (c) 2001-2011 The OpenSSL Project.  All rights reserved.
  *
@@ -2460,7 +2460,11 @@ aes_wrap_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
 	}
 
 	if (iv != NULL) {
-		memcpy(ctx->iv, iv, EVP_CIPHER_CTX_iv_length(ctx));
+		int iv_len = EVP_CIPHER_CTX_iv_length(ctx);
+
+		if (iv_len < 0 || iv_len > sizeof(ctx->iv))
+			return 0;
+		memcpy(ctx->iv, iv, iv_len);
 		wctx->iv = ctx->iv;
 	}
 
diff --git a/lib/libcrypto/evp/e_rc2.c b/lib/libcrypto/evp/e_rc2.c
index 32559e223f4..501e2dd31fe 100644
--- a/lib/libcrypto/evp/e_rc2.c
+++ b/lib/libcrypto/evp/e_rc2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: e_rc2.c,v 1.22 2023/07/07 19:37:53 beck Exp $ */
+/* $OpenBSD: e_rc2.c,v 1.23 2023/11/18 09:37:15 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -343,7 +343,7 @@ rc2_get_asn1_type_and_iv(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
 
 	if (type != NULL) {
 		l = EVP_CIPHER_CTX_iv_length(c);
-		if (l > sizeof(iv)) {
+		if (l < 0 || l > sizeof(iv)) {
 			EVPerror(EVP_R_IV_TOO_LARGE);
 			return -1;
 		}
@@ -373,6 +373,8 @@ rc2_set_asn1_type_and_iv(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
 	if (type != NULL) {
 		num = rc2_meth_to_magic(c);
 		j = EVP_CIPHER_CTX_iv_length(c);
+		if (j < 0 || j > sizeof(c->oiv))
+			return 0;
 		i = ASN1_TYPE_set_int_octetstring(type, num, c->oiv, j);
 	}
 	return (i);
@@ -381,9 +383,15 @@ rc2_set_asn1_type_and_iv(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
 static int
 rc2_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
 {
+	int iv_len;
+
 	switch (type) {
 	case EVP_CTRL_INIT:
-		data(c)->key_bits = EVP_CIPHER_CTX_key_length(c) * 8;
+		data(c)->key_bits = 0;
+		/* XXX - upper bound? */
+		if ((iv_len = EVP_CIPHER_CTX_key_length(c)) < 0)
+			return -1;
+		data(c)->key_bits = iv_len * 8;
 		return 1;
 
 	case EVP_CTRL_GET_RC2_KEY_BITS:
diff --git a/lib/libcrypto/evp/evp_enc.c b/lib/libcrypto/evp/evp_enc.c
index 7534b4c9d27..eb279b23784 100644
--- a/lib/libcrypto/evp/evp_enc.c
+++ b/lib/libcrypto/evp/evp_enc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: evp_enc.c,v 1.53 2023/09/10 16:53:56 tb Exp $ */
+/* $OpenBSD: evp_enc.c,v 1.54 2023/11/18 09:37:15 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -181,6 +181,8 @@ skip_to_init:
 	}
 
 	if (!(EVP_CIPHER_CTX_flags(ctx) & EVP_CIPH_CUSTOM_IV)) {
+		int iv_len;
+
 		switch (EVP_CIPHER_CTX_mode(ctx)) {
 
 		case EVP_CIPH_STREAM_CIPHER:
@@ -194,25 +196,26 @@ skip_to_init:
 			/* fall-through */
 
 		case EVP_CIPH_CBC_MODE:
-
-			if ((size_t)EVP_CIPHER_CTX_iv_length(ctx) >
-			    sizeof(ctx->iv)) {
+			iv_len = EVP_CIPHER_CTX_iv_length(ctx);
+			if (iv_len < 0 || iv_len > sizeof(ctx->oiv)) {
 				EVPerror(EVP_R_IV_TOO_LARGE);
 				return 0;
 			}
-			if (iv)
-				memcpy(ctx->oiv, iv,
-				    EVP_CIPHER_CTX_iv_length(ctx));
-			memcpy(ctx->iv, ctx->oiv,
-			    EVP_CIPHER_CTX_iv_length(ctx));
+			if (iv != NULL)
+				memcpy(ctx->oiv, iv, iv_len);
+			memcpy(ctx->iv, ctx->oiv, iv_len);
 			break;
 
 		case EVP_CIPH_CTR_MODE:
 			ctx->num = 0;
+			iv_len = EVP_CIPHER_CTX_iv_length(ctx);
+			if (iv_len < 0 || iv_len > sizeof(ctx->iv)) {
+				EVPerror(EVP_R_IV_TOO_LARGE);
+				return 0;
+			}
 			/* Don't reuse IV for CTR mode */
-			if (iv)
-				memcpy(ctx->iv, iv,
-				    EVP_CIPHER_CTX_iv_length(ctx));
+			if (iv != NULL)
+				memcpy(ctx->iv, iv, iv_len);
 			break;
 
 		default:
diff --git a/lib/libcrypto/evp/evp_lib.c b/lib/libcrypto/evp/evp_lib.c
index f4e46aea412..55573b21db5 100644
--- a/lib/libcrypto/evp/evp_lib.c
+++ b/lib/libcrypto/evp/evp_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: evp_lib.c,v 1.28 2023/09/28 11:29:10 tb Exp $ */
+/* $OpenBSD: evp_lib.c,v 1.29 2023/11/18 09:37:15 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -98,16 +98,16 @@ int
 EVP_CIPHER_get_asn1_iv(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
 {
 	int i = 0;
-	unsigned int l;
+	int l;
 
 	if (type != NULL) {
 		l = EVP_CIPHER_CTX_iv_length(c);
-		if (l > sizeof(c->iv)) {
+		if (l < 0 || l > sizeof(c->iv)) {
 			EVPerror(EVP_R_IV_TOO_LARGE);
 			return 0;
 		}
 		i = ASN1_TYPE_get_octetstring(type, c->oiv, l);
-		if (i != (int)l)
+		if (i != l)
 			return (-1);
 		else if (i > 0)
 			memcpy(c->iv, c->oiv, l);
@@ -119,11 +119,11 @@ int
 EVP_CIPHER_set_asn1_iv(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
 {
 	int i = 0;
-	unsigned int j;
+	int j;
 
 	if (type != NULL) {
 		j = EVP_CIPHER_CTX_iv_length(c);
-		if (j > sizeof(c->iv)) {
+		if (j < 0 || j > sizeof(c->iv)) {
 			EVPerror(EVP_R_IV_TOO_LARGE);
 			return 0;
 		}
diff --git a/lib/libcrypto/evp/p_seal.c b/lib/libcrypto/evp/p_seal.c
index b98da94360f..7f29ea0ca23 100644
--- a/lib/libcrypto/evp/p_seal.c
+++ b/lib/libcrypto/evp/p_seal.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: p_seal.c,v 1.16 2023/07/07 19:37:54 beck Exp $ */
+/* $OpenBSD: p_seal.c,v 1.17 2023/11/18 09:37:15 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -74,7 +74,7 @@ EVP_SealInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *type, unsigned char **ek,
     int *ekl, unsigned char *iv, EVP_PKEY **pubk, int npubk)
 {
 	unsigned char key[EVP_MAX_KEY_LENGTH];
-	int i;
+	int i, iv_len;
 
 	if (type) {
 		EVP_CIPHER_CTX_init(ctx);
@@ -85,8 +85,11 @@ EVP_SealInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *type, unsigned char **ek,
 		return 1;
 	if (EVP_CIPHER_CTX_rand_key(ctx, key) <= 0)
 		return 0;
-	if (EVP_CIPHER_CTX_iv_length(ctx))
-		arc4random_buf(iv, EVP_CIPHER_CTX_iv_length(ctx));
+	/* XXX - upper bound? */
+	if ((iv_len = EVP_CIPHER_CTX_iv_length(ctx)) < 0)
+		return 0;
+	if (iv_len > 0)
+		arc4random_buf(iv, iv_len);
 
 	if (!EVP_EncryptInit_ex(ctx, NULL, NULL, key, iv))
 		return 0;