OSSM-4294 Fixed buffer overrun in test CipherTest.cipher1

Signed-off-by: Ted Poole <tpoole@redhat.com>

bssl-compat/CMakeLists.txt

@@ -236,6 +236,7 @@ target_add_bssl_function(bssl-compat
   EVP_aes_256_cbc
   EVP_CIPHER_CTX_free
   EVP_CIPHER_CTX_new
+  EVP_CIPHER_block_size
   EVP_CIPHER_iv_length
   EVP_CIPHER_key_length
   EVP_DecryptFinal_ex

bssl-compat/patch/include/openssl/cipher.h.patch

@@ -117,7 +117,14 @@
 
  // EVP_Cipher performs a one-shot encryption/decryption operation for non-AEAD
  // ciphers. No partial blocks are maintained between calls. However, any
-@@ -324,11 +324,11 @@
+@@ -318,17 +318,17 @@
+
+ // EVP_CIPHER_block_size returns the block size, in bytes, for |cipher|, or one
+ // if |cipher| is a stream cipher.
+-// OPENSSL_EXPORT unsigned EVP_CIPHER_block_size(const EVP_CIPHER *cipher);
++OPENSSL_EXPORT unsigned EVP_CIPHER_block_size(const EVP_CIPHER *cipher);
+
+ // EVP_CIPHER_key_length returns the key size, in bytes, for |cipher|. If
  // |cipher| can take a variable key length then this function returns the
  // default key length and |EVP_CIPHER_flags| will return a value with
  // |EVP_CIPH_VARIABLE_LENGTH| set.
@@ -131,7 +138,7 @@
 
  // EVP_CIPHER_flags returns a value which is the OR of zero or more
  // |EVP_CIPH_*| flags.
-@@ -595,26 +595,26 @@
+@@ -601,26 +601,26 @@
  // } EVP_CIPHER_INFO;
 
 
@@ -168,7 +175,7 @@
 
  #ifdef ossl_CIPHER_R_AES_KEY_SETUP_FAILED
  #define CIPHER_R_AES_KEY_SETUP_FAILED ossl_CIPHER_R_AES_KEY_SETUP_FAILED
-@@ -695,4 +695,4 @@
+@@ -701,4 +701,4 @@
  #define CIPHER_R_INVALID_NONCE ossl_CIPHER_R_INVALID_NONCE
  #endif
 

bssl-compat/source/test/test_cipher.cc

@@ -1,53 +1,47 @@
 #include <gtest/gtest.h>
 #include <openssl/cipher.h>
+#include <openssl/rand.h>
 
-TEST(CipherTest, cipher1) {
-  uint8_t key[EVP_MAX_KEY_LENGTH] = {0};
-  uint8_t iv[EVP_MAX_IV_LENGTH] = {1};
-
-  uint8_t plaintext1[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
-                           1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
-                           1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
-                           1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
-  int plainlen1 = sizeof(plaintext1);
-
-  uint8_t plaintext2[sizeof(plaintext1)];
-  int plainlen2;
-
-  uint8_t ciphertext[1024];
-  int cipherlen;
-
-  int tmplen;
 
+TEST(CipherTest, cipher1) {
+  const EVP_CIPHER *cipher = EVP_aes_256_cbc();
 
-  const EVP_CIPHER *aes_256_cbc = EVP_aes_256_cbc();
+  ASSERT_TRUE(cipher);
 
-  EXPECT_TRUE(aes_256_cbc);
+  std::vector<uint8_t> key(EVP_CIPHER_key_length(cipher));
+  std::vector<uint8_t> iv(EVP_CIPHER_iv_length(cipher));
+
+  ASSERT_EQ(1, RAND_bytes(key.data(), key.size()));
+  ASSERT_EQ(1, RAND_bytes(iv.data(), iv.size()));
 
-  EXPECT_EQ(16, EVP_CIPHER_iv_length(aes_256_cbc));
-  EXPECT_EQ(32, EVP_CIPHER_key_length(aes_256_cbc));
+  std::vector<uint8_t> plaintext1 { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+                                    1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+                                    1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+                                    1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
+  std::vector<uint8_t> ciphertext;
+  std::vector<uint8_t> plaintext2;
 
   {
+    int l1, l2;
     bssl::UniquePtr<EVP_CIPHER_CTX> ctx(EVP_CIPHER_CTX_new());
 
-    EXPECT_EQ(1, EVP_EncryptInit_ex(ctx.get(), aes_256_cbc, nullptr, key, iv));
-    EXPECT_EQ(1, EVP_EncryptUpdate(ctx.get(), ciphertext, &cipherlen, plaintext1, plainlen1));
-    EXPECT_EQ(1, EVP_EncryptFinal_ex(ctx.get(), ciphertext + cipherlen, &tmplen));
-    cipherlen += tmplen;
+    ciphertext.resize(plaintext1.size() + EVP_CIPHER_block_size(cipher));
+    ASSERT_EQ(1, EVP_EncryptInit_ex(ctx.get(), cipher, nullptr, key.data(), iv.data()));
+    ASSERT_EQ(1, EVP_EncryptUpdate(ctx.get(), ciphertext.data(), &l1, plaintext1.data(), plaintext1.size()));
+    ASSERT_EQ(1, EVP_EncryptFinal_ex(ctx.get(), ciphertext.data() + l1, &l2));
+    ciphertext.resize(l1 + l2); // Resize to the actual encrypted byte count
   }
 
   {
+    int l1, l2;
     bssl::UniquePtr<EVP_CIPHER_CTX> ctx(EVP_CIPHER_CTX_new());
 
-    EXPECT_EQ(1, EVP_DecryptInit_ex(ctx.get(), aes_256_cbc, nullptr, key, iv));
-    EXPECT_EQ(1, EVP_DecryptUpdate(ctx.get(), plaintext2, &plainlen2, ciphertext, cipherlen));
-    EXPECT_EQ(1, EVP_DecryptFinal_ex(ctx.get(), plaintext2 + plainlen2, &tmplen));
-    plainlen2 += tmplen;
-  }
-
-  EXPECT_EQ(plainlen1, plainlen2);
-  for(auto i = 0; i < plainlen1; i++) {
-    EXPECT_EQ(plaintext1[i], plaintext2[i]);
+    plaintext2.resize(plaintext1.size() + EVP_CIPHER_block_size(cipher));
+    ASSERT_EQ(1, EVP_DecryptInit_ex(ctx.get(), cipher, nullptr, key.data(), iv.data()));
+    ASSERT_EQ(1, EVP_DecryptUpdate(ctx.get(), plaintext2.data(), &l1, ciphertext.data(), ciphertext.size()));
+    ASSERT_EQ(1, EVP_DecryptFinal_ex(ctx.get(), plaintext2.data() + l1, &l2));
+    plaintext2.resize(l1 + l2); // Resize to the actual decrypted byte count
   }
-}
 
+  ASSERT_EQ(plaintext1, plaintext2);
+}