SERVER-62922: Add explicit bounds checks for OpenSSL EVP outputs; linux-only unit tests

2 files changed, 121 insertions, 0 deletions

diff --git a/src/mongo/crypto/symmetric_crypto_openssl.cpp b/src/mongo/crypto/symmetric_crypto_openssl.cpp
index 42efc9a1d8d..c459ca3e4ab 100644
--- a/src/mongo/crypto/symmetric_crypto_openssl.cpp
+++ b/src/mongo/crypto/symmetric_crypto_openssl.cpp
@@ -84,6 +84,26 @@ public:
     }
 
     StatusWith<std::size_t> update(ConstDataRange in, DataRange out) final {
+        size_t cipherBlockSize = EVP_CIPHER_CTX_block_size(_ctx.get());
+
+
+        if (out.data() == nullptr) {
+            // Presumed intentional null output buffer
+            invariant(out.length() == 0);
+        } else {
+            // Data is padded to the next multiple of cipherBlockSize
+            size_t minimumOutputSize = in.length();
+            if (auto remainder = in.length() % cipherBlockSize) {
+                minimumOutputSize += cipherBlockSize - remainder;
+            }
+
+            if (out.length() < minimumOutputSize) {
+                return Status(ErrorCodes::Overflow,
+                              str::stream() << "Write buffer too small for Encryptor update: "
+                                            << static_cast<int>(out.length()));
+            }
+        }
+
         int len = 0;
         if (1 !=
             EVP_EncryptUpdate(
@@ -114,6 +134,14 @@ public:
     }
 
     StatusWith<std::size_t> finalize(DataRange out) final {
+
+        size_t cipherBlockSize = EVP_CIPHER_CTX_block_size(_ctx.get());
+
+        if (cipherBlockSize > 1 && out.length() < cipherBlockSize) {
+            return Status(ErrorCodes::Overflow,
+                          str::stream() << "Write buffer too small for Encryptor finalize: "
+                                        << static_cast<int>(out.length()));
+        }
         int len = 0;
         if (1 != EVP_EncryptFinal_ex(_ctx.get(), out.data<std::uint8_t>(), &len)) {
             return Status(ErrorCodes::UnknownError,
@@ -157,6 +185,25 @@ public:
 
     StatusWith<std::size_t> update(ConstDataRange in, DataRange out) final {
         int len = 0;
+
+        if (out.data() == nullptr) {
+            // Presumed intentional null output buffer
+            invariant(out.length() == 0);
+        } else {
+
+            size_t minimumOutputSize = in.length();
+            size_t cipherBlockSize = EVP_CIPHER_CTX_block_size(_ctx.get());
+            if (in.length() % cipherBlockSize) {
+                minimumOutputSize += cipherBlockSize;
+            }
+
+            if (out.length() < minimumOutputSize) {
+                return Status(ErrorCodes::Overflow,
+                              str::stream() << "Write buffer too small for Decryptor update: "
+                                            << static_cast<int>(out.length()));
+            }
+        }
+
         if (1 !=
             EVP_DecryptUpdate(
                 _ctx.get(), out.data<std::uint8_t>(), &len, in.data<std::uint8_t>(), in.length())) {
@@ -187,6 +234,14 @@ public:
 
     StatusWith<std::size_t> finalize(DataRange out) final {
         int len = 0;
+
+        size_t cipherBlockSize = EVP_CIPHER_CTX_block_size(_ctx.get());
+        if (cipherBlockSize > 1 && out.length() < cipherBlockSize) {
+            return Status(ErrorCodes::Overflow,
+                          str::stream() << "Write buffer too small for Encryptor finalize: "
+                                        << static_cast<int>(out.length()));
+        }
+
         if (1 != EVP_DecryptFinal_ex(_ctx.get(), out.data<std::uint8_t>(), &len)) {
             return Status(ErrorCodes::UnknownError,
                           str::stream()
diff --git a/src/mongo/crypto/symmetric_crypto_test.cpp b/src/mongo/crypto/symmetric_crypto_test.cpp
index 8f7acbd03dc..291145aa6dc 100644
--- a/src/mongo/crypto/symmetric_crypto_test.cpp
+++ b/src/mongo/crypto/symmetric_crypto_test.cpp
@@ -295,6 +295,72 @@ TEST(BlockPacker, AlignedThenOverfill) {
     ASSERT_EQ(1, leftovers.length());
 }
 
+#ifdef __linux__
+// (Only for OpenSSL, i.e. on Linux)
+// ... Try using insufficiently large output buffers for encryption and decryption
+TEST(SymmetricEncryptor, InsufficientOutputBuffer) {
+    SymmetricKey key = crypto::aesGenerate(crypto::sym256KeySize, "InsufficientOutputBufferTest");
+    constexpr auto plaintextMessage = "DOLOREM IPSUM"_sd;
+    std::vector<uint8_t> encodedPlaintext(plaintextMessage.begin(), plaintextMessage.end());
+    const std::array<uint8_t, 16> iv = {};
+    std::array<std::uint8_t, 1024> cryptoBuffer;
+    DataRange cryptoRange(cryptoBuffer.data(), cryptoBuffer.size());
+
+    auto swEnc = crypto::SymmetricEncryptor::create(key, crypto::aesMode::cbc, iv);
+    ASSERT_OK(swEnc.getStatus());
+    auto encryptor = std::move(swEnc.getValue());
+    DataRangeCursor cryptoCursor(cryptoRange);
+
+    // Validate that encryption with insufficient output buffer does not succeed
+    DataRange smallOutputBuffer(cryptoBuffer.data(), 1);
+    ASSERT_NOT_OK(encryptor->update(encodedPlaintext, smallOutputBuffer));
+
+    // Validate that encryption with zero output buffer does not succeed
+    DataRange zeroOutputBuffer(cryptoBuffer.data(), 0);
+    ASSERT_NOT_OK(
+        encryptor->update({plaintextMessage.rawData(), plaintextMessage.size()}, zeroOutputBuffer));
+
+    auto swSize = encryptor->update(encodedPlaintext, cryptoCursor);
+    ASSERT_OK(swSize);
+    cryptoCursor.advance(swSize.getValue());
+
+    swSize = encryptor->finalize(cryptoCursor);
+    ASSERT_OK(swSize);
+
+    // finalize is guaranteed to output at least 16 bytes for the CBC blockmode
+    ASSERT_GTE(swSize.getValue(), 16);
+    cryptoCursor.advance(swSize.getValue());
+
+    // Validate beginning of decryption process
+    auto swDec = crypto::SymmetricDecryptor::create(key, crypto::aesMode::cbc, iv);
+    ASSERT_OK(swDec.getStatus());
+    auto decryptor = std::move(swDec.getValue());
+
+    // Validate that decryption with insufficient output buffer does not succeed
+    std::array<uint8_t, 1> shortOutputBuffer;
+    DataRangeCursor shortOutputCursor(shortOutputBuffer);
+    ASSERT_NOT_OK(decryptor->update(
+        {cryptoRange.data(), cryptoRange.length() - cryptoCursor.length()}, shortOutputCursor));
+
+    // Validate that decryption with zero output buffer does not succeed
+    DataRangeCursor zeroOutputCursor(zeroOutputBuffer);
+    ASSERT_NOT_OK(decryptor->update(
+        {cryptoRange.data(), cryptoRange.length() - cryptoCursor.length()}, zeroOutputCursor));
+
+    // Validate that decryption update/finalize with sufficient output buffer succeeds
+    std::array<uint8_t, 1024> decryptionBuffer;
+    DataRangeCursor decryptionCursor(decryptionBuffer);
+    auto swUpdateSize = decryptor->update(
+        {cryptoRange.data(), cryptoRange.length() - cryptoCursor.length()}, decryptionCursor);
+    ASSERT_OK(swUpdateSize.getStatus());
+    decryptionCursor.advance(swUpdateSize.getValue());
+    auto swFinalizeSize = decryptor->finalize(decryptionCursor);
+    ASSERT_OK(swFinalizeSize.getStatus());
+
+    // Validate that the decrypted ciphertext matches the original plaintext
+    ASSERT(std::equal(plaintextMessage.begin(), plaintextMessage.end(), decryptionBuffer.begin()));
+}
+#endif
 
 // The following tests validate that SymmetricEncryptors function when called with inputs with
 // varying block alignments.