RC2: fix RC2 heap overflow with _huge_ key lengths.

There should be no way to trigger this from an attacker, as sane API use
should never allow users to set key lengths unchecked (or else one could
also set 8-bit key length effectively disabling encryption).

Change-Id: I8a1775f5b09ca1a620f95cdf51af55296a6a6964
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/95688
Commit-Queue: Rudolf Polzer <rpolzer@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/cipher/cipher_test.cc

diff --git a/crypto/cipher/cipher_test.cc b/crypto/cipher/cipher_test.cc
index 57b8636..ea593f5 100644
--- a/crypto/cipher/cipher_test.cc
+++ b/crypto/cipher/cipher_test.cc
@@ -1333,5 +1333,19 @@
   }
 }
 
+TEST(CipherTest, RC2KeyLengthOverflow) {
+  const EVP_CIPHER *cipher = EVP_rc2_cbc();
+  UniquePtr<EVP_CIPHER_CTX> ctx(EVP_CIPHER_CTX_new());
+  ASSERT_TRUE(ctx);
+
+  ASSERT_TRUE(EVP_DecryptInit_ex(ctx.get(), cipher, nullptr, nullptr, nullptr));
+
+  if (EVP_CIPHER_CTX_set_key_length(ctx.get(), 0x80000000)) {
+    std::vector<uint8_t> key(128, 0);
+    // This should not crash.
+    EVP_DecryptInit_ex(ctx.get(), nullptr, nullptr, key.data(), nullptr);
+  }
+}
+
 }  // namespace
 BSSL_NAMESPACE_END

crypto/cipher/e_rc2.cc

diff --git a/crypto/cipher/e_rc2.cc b/crypto/cipher/e_rc2.cc
index 91c5f52..21a3fc5 100644
--- a/crypto/cipher/e_rc2.cc
+++ b/crypto/cipher/e_rc2.cc
@@ -100,7 +100,7 @@
   uint16_t data[64];
 } RC2_KEY;
 
-static void RC2_encrypt(uint32_t *d, RC2_KEY *key) {
+static void rc2_encrypt(uint32_t *d, RC2_KEY *key) {
   int i, n;
   uint16_t *p0, *p1;
   uint16_t x0, x1, x2, x3, t;
@@ -144,7 +144,7 @@
   d[1] = (uint32_t)(x2 & 0xffff) | ((uint32_t)(x3 & 0xffff) << 16L);
 }
 
-static void RC2_decrypt(uint32_t *d, RC2_KEY *key) {
+static void rc2_decrypt(uint32_t *d, RC2_KEY *key) {
   int i, n;
   uint16_t *p0, *p1;
   uint16_t x0, x1, x2, x3, t;
@@ -189,7 +189,7 @@
   d[1] = (uint32_t)(x2 & 0xffff) | ((uint32_t)(x3 & 0xffff) << 16L);
 }
 
-static void RC2_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t length,
+static void rc2_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t length,
                             RC2_KEY *ks, uint8_t *iv, int encrypt) {
   uint32_t tin0, tin1;
   uint32_t tout0, tout1, xor0, xor1;
@@ -207,7 +207,7 @@
       tin1 ^= tout1;
       tin[0] = tin0;
       tin[1] = tin1;
-      RC2_encrypt(tin, ks);
+      rc2_encrypt(tin, ks);
       tout0 = tin[0];
       l2c(tout0, out);
       tout1 = tin[1];
@@ -219,7 +219,7 @@
       tin1 ^= tout1;
       tin[0] = tin0;
       tin[1] = tin1;
-      RC2_encrypt(tin, ks);
+      rc2_encrypt(tin, ks);
       tout0 = tin[0];
       l2c(tout0, out);
       tout1 = tin[1];
@@ -236,7 +236,7 @@
       tin[0] = tin0;
       c2l(in, tin1);
       tin[1] = tin1;
-      RC2_decrypt(tin, ks);
+      rc2_decrypt(tin, ks);
       tout0 = tin[0] ^ xor0;
       tout1 = tin[1] ^ xor1;
       l2c(tout0, out);
@@ -249,7 +249,7 @@
       tin[0] = tin0;
       c2l(in, tin1);
       tin[1] = tin1;
-      RC2_decrypt(tin, ks);
+      rc2_decrypt(tin, ks);
       tout0 = tin[0] ^ xor0;
       tout1 = tin[1] ^ xor1;
       l2cn(tout0, tout1, out, l + 8);
@@ -287,18 +287,23 @@
     0xfe, 0x7f, 0xc1, 0xad,
 };
 
-static void RC2_set_key(RC2_KEY *key, int len, const uint8_t *data, int bits) {
-  int i, j;
+static int rc2_set_key(RC2_KEY *key, size_t len, const uint8_t *data,
+                       int bits) {
+  size_t i, j;
   uint8_t *k;
-  uint16_t *ki;
   unsigned int c, d;
 
   k = (uint8_t *)&key->data[0];
   *k = 0;  // for if there is a zero length key
 
+  if (len == 0) {
+    OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BAD_KEY_LENGTH);
+    return 0;
+  }
   if (len > 128) {
     len = 128;
   }
+
   if (bits <= 0) {
     bits = 1024;
   }
@@ -326,16 +331,20 @@
 
   d = key_table[k[i] & c];
   k[i] = d;
-  while (i--) {
+
+  while (i > 0) {
+    --i;
     d = key_table[k[i + j] ^ d];
     k[i] = d;
   }
 
   // copy from bytes into uint16_t's
-  ki = &(key->data[63]);
-  for (i = 127; i >= 0; i -= 2) {
-    *(ki--) = ((k[i] << 8) | k[i - 1]) & 0xffff;
+  for (i = 0; i < 64; ++i) {
+    j = 63 - i;
+    key->data[j] = ((k[2 * j + 1] << 8) | k[2 * j]) & 0xffff;
   }
+
+  return 1;
 }
 
 typedef struct {
@@ -346,8 +355,10 @@
 static int rc2_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key,
                         const uint8_t *iv, int enc) {
   EVP_RC2_KEY *rc2_key = (EVP_RC2_KEY *)ctx->cipher_data;
-  RC2_set_key(&rc2_key->ks, EVP_CIPHER_CTX_key_length(ctx), key,
-              rc2_key->key_bits);
+  if (!rc2_set_key(&rc2_key->ks, EVP_CIPHER_CTX_key_length(ctx), key,
+                   rc2_key->key_bits)) {
+    return 0;
+  }
   return 1;
 }
 
@@ -357,13 +368,13 @@
   static const size_t kChunkSize = 0x10000;
 
   while (len >= kChunkSize) {
-    RC2_cbc_encrypt(in, out, kChunkSize, &key->ks, ctx->iv, ctx->encrypt);
+    rc2_cbc_encrypt(in, out, kChunkSize, &key->ks, ctx->iv, ctx->encrypt);
     len -= kChunkSize;
     in += kChunkSize;
     out += kChunkSize;
   }
   if (len) {
-    RC2_cbc_encrypt(in, out, len, &key->ks, ctx->iv, ctx->encrypt);
+    rc2_cbc_encrypt(in, out, len, &key->ks, ctx->iv, ctx->encrypt);
   }
   return 1;
 }