From 9ed0b76305ae73a3b121cccc74758a4e682089f1 Mon Sep 17 00:00:00 2001
From: MS-megliu <185960349+MS-megliu@users.noreply.github.com>
Date: Tue, 8 Jul 2025 17:05:35 -0700
Subject: [PATCH 1/4] CFB fix. change the blocksize to 1 for all CFBs (#131)

CFB fix. change the blocksize to 1 for all CFBs per
https://docs.openssl.org/3.0/man3/EVP_EncryptInit/#gettable-evp_cipher-parameters:~:text=%22blocksize%22%20(OSSL_CIPHER_PARAM_BLOCK_SIZE,the%20cached%20value.

test:
(1) test code from sql team passed.
(2) sslplay passed.
(3) buddy pipeline, functional test pipeline successfully.
---
 SymCryptProvider/src/ciphers/p_scossl_aes.c | 30 ++++++++++-----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/SymCryptProvider/src/ciphers/p_scossl_aes.c b/SymCryptProvider/src/ciphers/p_scossl_aes.c
index bfcf606d..e8b7adcd 100644
--- a/SymCryptProvider/src/ciphers/p_scossl_aes.c
+++ b/SymCryptProvider/src/ciphers/p_scossl_aes.c
@@ -970,7 +970,7 @@ static SCOSSL_STATUS scossl_aes_cfb8_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
     return p_scossl_aes_cfb_cipher_internal(ctx, 1, ctx->pbChainingValue, out, outl, outsize, in, inl);
 }
 
-#define IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(kbits, ivlen, lcmode, UCMODE, type)                           \
+#define IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(kbits, ivlen, lcmode, UCMODE, type, blocksize)                \
     SCOSSL_AES_CTX *p_scossl_aes_##kbits##_##lcmode##_newctx(_In_ SCOSSL_PROVCTX *provctx)                \
     {                                                                                                     \
         SCOSSL_COMMON_ALIGNED_ALLOC(ctx, OPENSSL_malloc, SCOSSL_AES_CTX);                                 \
@@ -990,7 +990,7 @@ static SCOSSL_STATUS scossl_aes_cfb8_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
     SCOSSL_STATUS p_scossl_aes_##kbits##_##lcmode##_get_params(_Inout_ OSSL_PARAM params[])               \
     {                                                                                                     \
         return p_scossl_aes_generic_get_params(params, EVP_CIPH_##UCMODE##_MODE, kbits >> 3,              \
-                                               ivlen, SYMCRYPT_AES_BLOCK_SIZE, 0);                        \
+                                               ivlen, blocksize, 0);                                      \
     }                                                                                                     \
                                                                                                           \
     const OSSL_DISPATCH p_scossl_aes##kbits##lcmode##_functions[] = {                                     \
@@ -999,7 +999,7 @@ static SCOSSL_STATUS scossl_aes_cfb8_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
         {OSSL_FUNC_CIPHER_FREECTX, (void (*)(void))p_scossl_aes_generic_freectx},                         \
         {OSSL_FUNC_CIPHER_ENCRYPT_INIT, (void (*)(void))p_scossl_aes_generic_encrypt_init},               \
         {OSSL_FUNC_CIPHER_DECRYPT_INIT, (void (*)(void))p_scossl_aes_generic_decrypt_init},               \
-        {OSSL_FUNC_CIPHER_UPDATE, (void (*)(void))p_scossl_aes_generic_##type##_update},                \
+        {OSSL_FUNC_CIPHER_UPDATE, (void (*)(void))p_scossl_aes_generic_##type##_update},                  \
         {OSSL_FUNC_CIPHER_FINAL, (void (*)(void))p_scossl_aes_generic_##type##_final},                    \
         {OSSL_FUNC_CIPHER_CIPHER, (void (*)(void))p_scossl_aes_generic_cipher},                           \
         {OSSL_FUNC_CIPHER_GET_PARAMS, (void (*)(void))p_scossl_aes_##kbits##_##lcmode##_get_params},      \
@@ -1010,21 +1010,21 @@ static SCOSSL_STATUS scossl_aes_cfb8_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
         {OSSL_FUNC_CIPHER_SETTABLE_CTX_PARAMS, (void (*)(void))p_scossl_aes_generic_settable_ctx_params}, \
         {0, NULL}};
 
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, SYMCRYPT_AES_BLOCK_SIZE, cbc, CBC, block)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, SYMCRYPT_AES_BLOCK_SIZE, cbc, CBC, block)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, SYMCRYPT_AES_BLOCK_SIZE, cbc, CBC, block)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, SYMCRYPT_AES_BLOCK_SIZE, cbc, CBC, block, SYMCRYPT_AES_BLOCK_SIZE)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, SYMCRYPT_AES_BLOCK_SIZE, cbc, CBC, block, SYMCRYPT_AES_BLOCK_SIZE)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, SYMCRYPT_AES_BLOCK_SIZE, cbc, CBC, block, SYMCRYPT_AES_BLOCK_SIZE)
 
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, 0, ecb, ECB, block)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, 0, ecb, ECB, block)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, 0, ecb, ECB, block)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, 0, ecb, ECB, block, SYMCRYPT_AES_BLOCK_SIZE)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, 0, ecb, ECB, block, SYMCRYPT_AES_BLOCK_SIZE)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, 0, ecb, ECB, block, SYMCRYPT_AES_BLOCK_SIZE)
 
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, SYMCRYPT_AES_BLOCK_SIZE, cfb, CFB, stream)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, SYMCRYPT_AES_BLOCK_SIZE, cfb, CFB, stream)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, SYMCRYPT_AES_BLOCK_SIZE, cfb, CFB, stream)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, SYMCRYPT_AES_BLOCK_SIZE, cfb, CFB, stream, 1)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, SYMCRYPT_AES_BLOCK_SIZE, cfb, CFB, stream, 1)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, SYMCRYPT_AES_BLOCK_SIZE, cfb, CFB, stream, 1)
 
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, SYMCRYPT_AES_BLOCK_SIZE, cfb8, CFB, stream)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, SYMCRYPT_AES_BLOCK_SIZE, cfb8, CFB, stream)
-IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, SYMCRYPT_AES_BLOCK_SIZE, cfb8, CFB, stream)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(128, SYMCRYPT_AES_BLOCK_SIZE, cfb8, CFB, stream, 1)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(192, SYMCRYPT_AES_BLOCK_SIZE, cfb8, CFB, stream, 1)
+IMPLEMENT_SCOSSL_AES_GENERIC_CIPHER(256, SYMCRYPT_AES_BLOCK_SIZE, cfb8, CFB, stream, 1)
 
 #ifdef __cplusplus
 }

From e3dd13a32d77c57f85d6639942183165268f462f Mon Sep 17 00:00:00 2001
From: Samuel Lee <56448320+samuel-lee-msft@users.noreply.github.com>
Date: Wed, 9 Jul 2025 17:18:07 -0700
Subject: [PATCH 2/4] Fix for TLS 1.x CBC ciphersuites (#130)

Builds on fix: compatibility issue between SCOSSL and TLS 1.2 ciphersuites which use HMAC in AzL3 #129 but handles overly large padding on decryption

Addresses bug 58142883
Error Message:
openssl s_client -connect tcs.microsoftstore.com.cn:443 -tls1_2 -cipher ECDHE-RSA-AES256-SHA384
40D79C8E857B0000:error:0A0003FC:SSL routines:ssl3_read_bytes:ssl/tls alert bad record mac:ssl/record/rec_layer_s3.c:907:SSL alert number 20

Root cause:
(1) TLS1.2 uses PKCS7-like padding for CBC cipher modes, but it is not PKCS7. SCOSSL was using PKCS7 padding incorrectly in TLS CBC paths, both causing errors on encryption path (other party would expect padding to be one byte longer and normally fail in padding check, if not in subsequent MAC check), and the decryption path (one byte of other party's padding would be interpreted as the last byte of the MAC)
(2) copy_mac didn't copy the correct mac for aes get parameter to consume, caused bad mac in decryption path.
(3) fix typo in 1.10 for public key export

Test:
(1) sslplay passed
(2) openssl s_client -connect tcs.microsoftstore.com.cn:443 -tls1_2 -cipher ECDHE-RSA-AES256-SHA384 worked.
---
 SymCryptProvider/src/ciphers/p_scossl_aes.c   | 161 ++++++++++--------
 .../src/keymgmt/p_scossl_ecc_keymgmt.c        |   2 +-
 2 files changed, 92 insertions(+), 71 deletions(-)

diff --git a/SymCryptProvider/src/ciphers/p_scossl_aes.c b/SymCryptProvider/src/ciphers/p_scossl_aes.c
index e8b7adcd..984e8bbe 100644
--- a/SymCryptProvider/src/ciphers/p_scossl_aes.c
+++ b/SymCryptProvider/src/ciphers/p_scossl_aes.c
@@ -150,43 +150,49 @@ static SCOSSL_STATUS p_scossl_aes_generic_decrypt_init(_Inout_ SCOSSL_AES_CTX *c
 
 #define SYMCRYPT_OPENSSL_MASK8_SELECT( _mask, _a, _b ) (SYMCRYPT_FORCE_READ8(&_mask) & _a) | (~(SYMCRYPT_FORCE_READ8(&_mask)) & _b)
 
-// Extracts the MAC from the end of out and saves the result to ctx->tlsMac
-// The mac will later be fetched through p_scossl_aes_generic_get_ctx_params
-// This function is adapted from ssl3_cbc_copy_mac in ssl/record/tls_pad.c
-// and runs in constant time. In case of bad padding, a random MAC is assigned instead
-static SCOSSL_STATUS p_scossl_aes_copy_mac(_Inout_ SCOSSL_AES_CTX *ctx,
-                                           _In_reads_bytes_(buflen) unsigned char *record, _Inout_ SIZE_T *recordLen,
-                                           SIZE_T recordLenPadded,
-                                           SCOSSL_STATUS paddingStatus)
+// Verifies the TLS padding from the end of record, extracts the MAC from the end of
+// the unpadded record, and saves the result to ctx->tlsMac.
+//
+// The MAC will later be fetched through p_scossl_aes_generic_get_ctx_params
+// This function is adapted from ssl3_cbc_copy_mac in ssl/record/tls_pad.c, and 
+// SymCryptTlsCbcHmacVerifyCore from SymCrypt, and runs in constant time w.r.t
+// the values in pbData. In case of bad padding, a random MAC is assigned instead
+static SCOSSL_STATUS p_scossl_aes_tls_remove_padding_and_copy_mac(
+    _Inout_ SCOSSL_AES_CTX *ctx,
+    _In_reads_bytes_(*pcbData) unsigned char *pbData,
+    _Inout_ SIZE_T *pcbData)
 {
+    SIZE_T cbDataOrig = *pcbData;
+    unsigned const char * pbTail = pbData;
+    UINT32 cbTail = (UINT32)cbDataOrig;
+    UINT32 u32;
+    UINT32 cbPad;
+    UINT32 maxPadLength;
+
     // MAC rotation is performed in place
     BYTE rotatedMacBuf[64 + EVP_MAX_MD_SIZE];
     PBYTE rotatedMac;
-    BYTE aux1, aux2, aux3, mask;
-    BYTE paddingStatusByte = (BYTE) (paddingStatus & 0xff);
-    // Random mac set in case padding removal failed
     BYTE randMac[EVP_MAX_MD_SIZE];
+    BYTE paddingStatus = 0; // 0x00 for valid padding, 0xff for bad padding
 
-    UINT32 macEnd = *record;
-    UINT32 macStart = macEnd - ctx->tlsMacSize;
+    UINT32 macEnd; // index in pbTail
+    UINT32 macStart; // index in pbTail
     UINT32 inMac = 0;
 
-    UINT32 scanStart = 0;
     UINT32 rotateOffset = 0;
     UINT32 i, j;
 
     OPENSSL_free(ctx->tlsMac);
     ctx->tlsMac = NULL;
 
-    // Public info, safe to branch
-    // No mac to copy
-    if (ctx->tlsMacSize == 0)
+    // Check that we have enough data for a valid record.
+    // We need one MAC value plus one padding_length byte
+    if (cbDataOrig < ctx->tlsMacSize + 1)
     {
-        return paddingStatus;
+        ERR_raise(ERR_LIB_PROV, PROV_R_CIPHER_OPERATION_FAILED);
+        return SCOSSL_FAILURE;
     }
 
-    *recordLen -= ctx->tlsMacSize;
-
     // Generate random bytes in case of bad padding
     if (RAND_bytes_ex(ctx->libctx, randMac, ctx->tlsMacSize, 0) <= 0)
     {
@@ -199,46 +205,83 @@ static SCOSSL_STATUS p_scossl_aes_copy_mac(_Inout_ SCOSSL_AES_CTX *ctx,
         return SCOSSL_FAILURE;
     }
 
-    rotatedMac = rotatedMacBuf + ((0 - (SIZE_T)rotatedMacBuf) & 0x3f);
-
-    // Public info, safe to branch
-    if (recordLenPadded > ctx->tlsMacSize + 255 + 1)
+    // We only care about the tail of the input buffer, which we can index with UINT32 indices
+    // The if() is safe as both cbData and u32 are public values.
+    u32 = ctx->tlsMacSize + 255 + 1;
+    if( cbDataOrig > u32 )
     {
-        scanStart = recordLenPadded - (ctx->tlsMacSize + 255 + 1);
+        pbTail += cbDataOrig - u32;
+        cbTail = u32;
     }
 
-    // Find and extract MAC
+    // Pick up the padding_length. Note that this is the value we have to keep secret from
+    // side-channel attacks.
+    cbPad = pbTail[cbTail - 1]; // cbPad in range [0,255]
+
+    // Bound the padding length to cbTail - tlsMacSize
+    // This doesn't reveal data as we treat all cbPad values the same, but it makes our
+    // further computations easier
+    maxPadLength = (UINT32)cbTail - ctx->tlsMacSize;    // We checked this is >= 0
+    u32 = SYMCRYPT_MASK32_LT( maxPadLength, cbPad );    // mask: maxPadLength < cbPad
+    cbPad = cbPad + ((maxPadLength - cbPad) & u32);
+    paddingStatus |= (BYTE)u32; // validation fails if the padding would overlap with the MAC
+
+    macEnd = (cbTail - 1) - cbPad;
+    macStart = macEnd - ctx->tlsMacSize;
+
+    rotatedMac = rotatedMacBuf + ((0 - (SIZE_T)rotatedMacBuf) & 0x3f);
+    
+    // Find and extract MAC, and verify padding
     memset(rotatedMac, 0, ctx->tlsMacSize);
-    for (i = scanStart, j = 0; i < recordLenPadded; i++)
+    for (i = 0, j = 0; i < cbTail-1; i++)
     {
         UINT32 macStarted = SYMCRYPT_MASK32_EQ(i, macStart);
-        UINT32 macEnded = SYMCRYPT_MASK32_LT(i, macEnd);
-        BYTE recordByte = record[i];
+        UINT32 macNotEnded = SYMCRYPT_MASK32_LT(i, macEnd);
+        BYTE recordByte = pbTail[i];
 
-        inMac = (inMac | macStarted) & macEnded;
+        inMac = (inMac | macStarted) & macNotEnded;
         rotateOffset |= j & macStarted;
         rotatedMac[j++] |= recordByte & inMac;
         j &= SYMCRYPT_MASK32_LT(j, ctx->tlsMacSize);
+
+        paddingStatus |= (BYTE)((~SYMCRYPT_MASK32_EQ(recordByte, cbPad)) & (~macNotEnded));
     }
 
     // MAC rotation
-    for (i = 0, j = 0; i < ctx->tlsMacSize; i++)
+    for (i = 0; i < ctx->tlsMacSize; i++)
     {
-        // in case cache-line is 32 bytes, load from both lines and select appropriately
-        aux1 = rotatedMac[rotateOffset & ~0x20];
-        aux2 = rotatedMac[rotateOffset | 0x20];
-        mask = (BYTE) SYMCRYPT_MASK32_EQ(rotateOffset & !0x20, rotateOffset);
-        aux3 = SYMCRYPT_OPENSSL_MASK8_SELECT(mask, aux1, aux2);
+        BYTE macByte = 0;
+        for (j = 0; j < ctx->tlsMacSize; j++) {
+            UINT32 match = SYMCRYPT_MASK32_EQ(j, (rotateOffset + i) % ctx->tlsMacSize);
+            macByte |= rotatedMac[j] & match;
+        }
+        ctx->tlsMac[i] = SYMCRYPT_OPENSSL_MASK8_SELECT(paddingStatus, randMac[i], macByte);
+    }
 
+    *pcbData -= (1 + cbPad + ctx->tlsMacSize);
 
-        ctx->tlsMac[j++] = SYMCRYPT_OPENSSL_MASK8_SELECT(paddingStatusByte, aux3, randMac[i]);
+    return SCOSSL_SUCCESS;
+}
+
+static SCOSSL_STATUS p_scossl_aes_tls_add_padding(const unsigned char *in, size_t inl, unsigned char *out, size_t outsize, size_t *outlen)
+{
+    // TLS padding with 1-16 bytes, each with value (cbPad-1)
+    SIZE_T cbPad = SYMCRYPT_AES_BLOCK_SIZE - (inl & (SYMCRYPT_AES_BLOCK_SIZE-1));
+
+    if (inl + cbPad > outsize)
+    {
+        ERR_raise(ERR_LIB_PROV, PROV_R_OUTPUT_BUFFER_TOO_SMALL);
+        return SCOSSL_FAILURE; // Buffer too small
+    }
 
-        rotateOffset++;
-        rotateOffset = (rotateOffset & SYMCRYPT_MASK32_LT(rotateOffset, ctx->tlsMacSize));
+    if (in != out)
+    {
+        memmove(out, in, inl);
     }
 
-    // If we failed, we still succeed, but the MAC is set to some
-    // random value. It's up to the caller to check the MAC.
+    memset(out + inl, (unsigned char)(cbPad - 1), cbPad);
+    *outlen = inl + cbPad;
+
     return SCOSSL_SUCCESS;
 }
 
@@ -269,10 +312,10 @@ static SCOSSL_STATUS p_scossl_aes_generic_block_update(_Inout_ SCOSSL_AES_CTX *c
         if (ctx->encrypt)
         {
             // in == out
-            SymCryptPaddingPkcs7Add(
-                SYMCRYPT_AES_BLOCK_SIZE,
-                in, inl,
-                out, outsize, &inl);
+            if (p_scossl_aes_tls_add_padding(in, inl, out, outsize, &inl) != SCOSSL_SUCCESS)
+            {
+                return SCOSSL_FAILURE;
+            }
         }
 
         if (inl % SYMCRYPT_AES_BLOCK_SIZE != 0 ||
@@ -282,18 +325,12 @@ static SCOSSL_STATUS p_scossl_aes_generic_block_update(_Inout_ SCOSSL_AES_CTX *c
             return SCOSSL_FAILURE;
         }
 
-        // Need to remove padding and mac in constant time
+        // Need to remove TLS padding and MAC in constant time
         if (!ctx->encrypt)
         {
-            SYMCRYPT_ERROR scError = SYMCRYPT_NO_ERROR;
-            // Return SCOSSL_FAILURE for any code that isn't SYMCRYPT_NO_ERROR
-            SYMCRYPT_UINT32_MAP scErrorMap[1] = {
-                {SYMCRYPT_NO_ERROR, SCOSSL_SUCCESS}};
-            SIZE_T outlPadded;
-
             switch (ctx->tlsVersion)
             {
-            // Need to remove explicit IV in addition to pkcs7 padding and mac
+            // Need to remove explicit IV in addition to TLS padding and MAC
             case TLS1_2_VERSION:
             case DTLS1_2_VERSION:
             case TLS1_1_VERSION:
@@ -303,23 +340,7 @@ static SCOSSL_STATUS p_scossl_aes_generic_block_update(_Inout_ SCOSSL_AES_CTX *c
                 *outl -= SYMCRYPT_AES_BLOCK_SIZE;
                 __attribute__ ((fallthrough));
             case TLS1_VERSION:
-                outlPadded = *outl;
-
-                if (ctx->tlsMacSize > *outl)
-                {
-                    ERR_raise(ERR_LIB_PROV, PROV_R_CIPHER_OPERATION_FAILED);
-                    return SCOSSL_FAILURE;
-                }
-                scError = SymCryptPaddingPkcs7Remove(
-                    SYMCRYPT_AES_BLOCK_SIZE,
-                    out, *outl,
-                    out, *outl,
-                    outl);
-
-                return p_scossl_aes_copy_mac(ctx,
-                                             out, outl,
-                                             outlPadded,
-                                             SymCryptMapUint32(scError, SCOSSL_FAILURE, scErrorMap, 1));
+                return p_scossl_aes_tls_remove_padding_and_copy_mac(ctx, out, outl);
                 break;
             default:
                 ERR_raise(ERR_LIB_PROV, PROV_R_CIPHER_OPERATION_FAILED);
diff --git a/SymCryptProvider/src/keymgmt/p_scossl_ecc_keymgmt.c b/SymCryptProvider/src/keymgmt/p_scossl_ecc_keymgmt.c
index 741bb8f3..c3a2f7e6 100644
--- a/SymCryptProvider/src/keymgmt/p_scossl_ecc_keymgmt.c
+++ b/SymCryptProvider/src/keymgmt/p_scossl_ecc_keymgmt.c
@@ -390,7 +390,7 @@ static SCOSSL_STATUS p_scossl_ecc_keymgmt_get_params(_In_ SCOSSL_ECC_KEY_CTX *ke
     if ((p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY)) != NULL)
     {
         SIZE_T cbEncodedKey;
-        if (!p_scossl_ecc_get_encoded_key(keyCtx, OSSL_KEYMGMT_SELECT_PRIVATE_KEY, &pbEncodedKey, &cbEncodedKey) ||
+        if (!p_scossl_ecc_get_encoded_key(keyCtx, OSSL_KEYMGMT_SELECT_PUBLIC_KEY, &pbEncodedKey, &cbEncodedKey) ||
             !OSSL_PARAM_set_octet_string(p, pbEncodedKey, cbEncodedKey))
         {
             ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER);

From 8a6a595d5e7d3187a67312771647e1ec9f82365a Mon Sep 17 00:00:00 2001
From: Samuel Lee <56448320+samuel-lee-msft@users.noreply.github.com>
Date: Thu, 10 Jul 2025 17:53:32 -0700
Subject: [PATCH 3/4] Fix use of uninitialized memory and consistency of code
 handling allocations (#133)

* Refactor logic to avoid use of uninitialized memory and improve
consistency

* Fix up a few more nitpicks

+ Potential memory leak in common gcm init ctx (never hit with provider,
  but could be hit from engine)
+ Consistency of dupctx ERR and assignment

* Fix some unhandled OPENSSL_strdup failures
---
 ScosslCommon/src/scossl_aes_aead.c            | 11 +++--
 ScosslCommon/src/scossl_mac.c                 |  7 +++-
 ScosslCommon/src/scossl_tls1prf.c             | 19 ++++-----
 .../src/ciphers/p_scossl_aes_aead.c           |  4 +-
 .../src/ciphers/p_scossl_aes_xts.c            |  2 +-
 .../src/digests/p_scossl_cshake.c             | 40 ++++--------------
 SymCryptProvider/src/kdf/p_scossl_hkdf.c      |  9 ++--
 SymCryptProvider/src/kdf/p_scossl_pbkdf2.c    | 13 ++----
 SymCryptProvider/src/kdf/p_scossl_srtpkdf.c   | 21 ++--------
 SymCryptProvider/src/kdf/p_scossl_sshkdf.c    | 42 ++++++++++++++-----
 SymCryptProvider/src/kdf/p_scossl_tls1prf.c   | 39 +++++++++++++----
 SymCryptProvider/src/kem/p_scossl_mlkem.c     |  8 ++--
 SymCryptProvider/src/keyexch/p_scossl_dh.c    |  6 +--
 SymCryptProvider/src/keyexch/p_scossl_ecdh.c  |  4 +-
 .../src/keymgmt/p_scossl_dh_keymgmt.c         | 18 ++++----
 .../src/keymgmt/p_scossl_rsa_keymgmt.c        |  2 +-
 SymCryptProvider/src/mac/p_scossl_hmac.c      |  6 ++-
 SymCryptProvider/src/p_scossl_ecc.c           |  2 +-
 18 files changed, 128 insertions(+), 125 deletions(-)

diff --git a/ScosslCommon/src/scossl_aes_aead.c b/ScosslCommon/src/scossl_aes_aead.c
index 61fe1e32..b48bfe58 100644
--- a/ScosslCommon/src/scossl_aes_aead.c
+++ b/ScosslCommon/src/scossl_aes_aead.c
@@ -21,9 +21,14 @@ SCOSSL_STATUS scossl_aes_gcm_init_ctx(SCOSSL_CIPHER_GCM_CTX *ctx, const unsigned
     ctx->useInvocation = 0;
     ctx->ivlen = SCOSSL_GCM_DEFAULT_IV_LENGTH;
 
-    if (iv != NULL && (ctx->iv = OPENSSL_memdup(iv, ctx->ivlen)) == NULL)
+    if (iv != NULL)
     {
-        return SCOSSL_FAILURE;
+        OPENSSL_free(ctx->iv);
+
+        if ((ctx->iv = OPENSSL_memdup(iv, ctx->ivlen)) == NULL)
+        {
+            return SCOSSL_FAILURE;
+        }
     }
 
     return SCOSSL_SUCCESS;
@@ -431,7 +436,7 @@ void scossl_aes_ccm_init_ctx(SCOSSL_CIPHER_CCM_CTX *ctx,
                              const unsigned char *iv)
 {
     ctx->ivlen = SCOSSL_CCM_MIN_IV_LENGTH;
-    if (iv)
+    if (iv != NULL)
     {
         memcpy(ctx->iv, iv, ctx->ivlen);
     }
diff --git a/ScosslCommon/src/scossl_mac.c b/ScosslCommon/src/scossl_mac.c
index fae4f48b..f217f1c0 100644
--- a/ScosslCommon/src/scossl_mac.c
+++ b/ScosslCommon/src/scossl_mac.c
@@ -129,7 +129,12 @@ SCOSSL_MAC_CTX *scossl_mac_dupctx(SCOSSL_MAC_CTX *ctx)
             }
         }
 
-        copyCtx->mdName = OPENSSL_strdup(ctx->mdName);
+        if (ctx->mdName != NULL &&
+            (copyCtx->mdName = OPENSSL_strdup(ctx->mdName)) == NULL)
+        {
+            goto cleanup;
+        }
+
         copyCtx->libctx = ctx->libctx;
     }
 
diff --git a/ScosslCommon/src/scossl_tls1prf.c b/ScosslCommon/src/scossl_tls1prf.c
index 15678a58..9d045026 100644
--- a/ScosslCommon/src/scossl_tls1prf.c
+++ b/ScosslCommon/src/scossl_tls1prf.c
@@ -17,23 +17,18 @@ _Use_decl_annotations_
 SCOSSL_TLS1_PRF_CTX *scossl_tls1prf_dupctx(SCOSSL_TLS1_PRF_CTX *ctx)
 {
     SCOSSL_TLS1_PRF_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_TLS1_PRF_CTX));
+
     if (copyCtx != NULL)
     {
-        if (ctx->pbSecret == NULL)
-        {
-            copyCtx->pbSecret = NULL;
-        }
-        else if ((copyCtx->pbSecret = OPENSSL_memdup(ctx->pbSecret, ctx->cbSecret)) == NULL)
+        *copyCtx = *ctx;
+        copyCtx->pbSecret = NULL;
+        
+        if (ctx->pbSecret != NULL &&
+            (copyCtx->pbSecret = OPENSSL_memdup(ctx->pbSecret, ctx->cbSecret)) == NULL)
         {
             scossl_tls1prf_freectx(copyCtx);
-            return NULL;
+            copyCtx = NULL;
         }
-
-        copyCtx->isTlsPrf1_1 = ctx->isTlsPrf1_1;
-        copyCtx->pHmac = ctx->pHmac;
-        copyCtx->cbSecret = ctx->cbSecret;
-        copyCtx->cbSeed = ctx->cbSeed;
-        memcpy(copyCtx->seed, ctx->seed, ctx->cbSeed);
     }
 
     return copyCtx;
diff --git a/SymCryptProvider/src/ciphers/p_scossl_aes_aead.c b/SymCryptProvider/src/ciphers/p_scossl_aes_aead.c
index 98e4471f..531ba344 100644
--- a/SymCryptProvider/src/ciphers/p_scossl_aes_aead.c
+++ b/SymCryptProvider/src/ciphers/p_scossl_aes_aead.c
@@ -65,10 +65,11 @@ static SCOSSL_CIPHER_GCM_CTX *p_scossl_aes_gcm_dupctx(_In_ SCOSSL_CIPHER_GCM_CTX
     SCOSSL_COMMON_ALIGNED_ALLOC(copy_ctx, OPENSSL_malloc, SCOSSL_CIPHER_GCM_CTX);
     if (copy_ctx != NULL)
     {
-        memcpy(copy_ctx, ctx, sizeof(SCOSSL_CIPHER_GCM_CTX));
+        *copy_ctx = *ctx;
 
         if (ctx->iv != NULL && (copy_ctx->iv = OPENSSL_memdup(ctx->iv, ctx->ivlen)) == NULL)
         {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
             p_scossl_aes_gcm_freectx(copy_ctx);
             return NULL;
         }
@@ -552,7 +553,6 @@ static SCOSSL_STATUS p_scossl_aes_ccm_set_ctx_params(_Inout_ SCOSSL_CIPHER_CCM_C
         if (ctx != NULL)                                                                                     \
         {                                                                                                    \
             ctx->keylen = kbits >> 3;                                                                        \
-            ctx->ivlen = defaultIvLen;                                                                       \
             scossl_aes_##lcmode##_init_ctx(ctx, NULL);                                                       \
         }                                                                                                    \
                                                                                                              \
diff --git a/SymCryptProvider/src/ciphers/p_scossl_aes_xts.c b/SymCryptProvider/src/ciphers/p_scossl_aes_xts.c
index a1b6961b..1d38e3a3 100644
--- a/SymCryptProvider/src/ciphers/p_scossl_aes_xts.c
+++ b/SymCryptProvider/src/ciphers/p_scossl_aes_xts.c
@@ -40,7 +40,7 @@ static SCOSSL_STATUS p_scossl_aes_xts_set_ctx_params(_Inout_ SCOSSL_AES_XTS_CTX
 
 static SCOSSL_AES_XTS_CTX *p_scossl_aes_xts_newctx_internal(size_t keylen)
 {
-    SCOSSL_COMMON_ALIGNED_ALLOC(ctx, OPENSSL_malloc, SCOSSL_AES_XTS_CTX);
+    SCOSSL_COMMON_ALIGNED_ALLOC(ctx, OPENSSL_zalloc, SCOSSL_AES_XTS_CTX);
     if (ctx != NULL)
     {
         ctx->keylen = keylen;
diff --git a/SymCryptProvider/src/digests/p_scossl_cshake.c b/SymCryptProvider/src/digests/p_scossl_cshake.c
index fc5abe17..b3f58389 100644
--- a/SymCryptProvider/src/digests/p_scossl_cshake.c
+++ b/SymCryptProvider/src/digests/p_scossl_cshake.c
@@ -132,43 +132,21 @@ static SCOSSL_CSHAKE_CTX *p_scossl_cshake_dupctx(_In_ SCOSSL_CSHAKE_CTX *ctx)
 
     SCOSSL_COMMON_ALIGNED_ALLOC(copyCtx, OPENSSL_zalloc, SCOSSL_CSHAKE_CTX);
 
-    if (ctx != NULL)
+    if (copyCtx != NULL)
     {
-        if (ctx->pbFunctionNameString != NULL)
-        {
-            copyCtx->pbFunctionNameString = OPENSSL_memdup(ctx->pbFunctionNameString, ctx->cbFunctionNameString);
-            if (copyCtx->pbFunctionNameString == NULL)
-            {
-                ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
-                goto cleanup;
-            }
-        }
-        else
-        {
-            copyCtx->pbFunctionNameString = NULL;
-        }
-        copyCtx->cbFunctionNameString = ctx->cbFunctionNameString;
+        *copyCtx = *ctx;
 
-        if (ctx->pbCustomizationString != NULL)
-        {
-            copyCtx->pbCustomizationString = OPENSSL_memdup(ctx->pbCustomizationString, ctx->cbCustomizationString);
-            if (copyCtx->pbCustomizationString == NULL)
-            {
-                ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
-                goto cleanup;
-            }
-        }
-        else
+        copyCtx->pbFunctionNameString = OPENSSL_memdup(ctx->pbFunctionNameString, ctx->cbFunctionNameString);
+        copyCtx->pbCustomizationString = OPENSSL_memdup(ctx->pbCustomizationString, ctx->cbCustomizationString);
+
+        if ((ctx->pbFunctionNameString != NULL  && copyCtx->pbFunctionNameString == NULL) ||
+            (ctx->pbCustomizationString != NULL && copyCtx->pbCustomizationString == NULL))
         {
-            copyCtx->pbCustomizationString = NULL;
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
         }
-        copyCtx->cbCustomizationString = ctx->cbCustomizationString;
 
         ctx->pHash->stateCopyFunc(&ctx->state, &copyCtx->state);
-
-        copyCtx->pHash = ctx->pHash;
-        copyCtx->xofState = ctx->xofState;
-        copyCtx->xofLen = ctx->xofLen;
     }
 
     status = SCOSSL_SUCCESS;
diff --git a/SymCryptProvider/src/kdf/p_scossl_hkdf.c b/SymCryptProvider/src/kdf/p_scossl_hkdf.c
index 59202e0f..1c5ec3d0 100644
--- a/SymCryptProvider/src/kdf/p_scossl_hkdf.c
+++ b/SymCryptProvider/src/kdf/p_scossl_hkdf.c
@@ -62,12 +62,11 @@ SCOSSL_PROV_HKDF_CTX *p_scossl_hkdf_newctx(_In_ SCOSSL_PROVCTX *provctx)
 
 void p_scossl_hkdf_freectx(_Inout_ SCOSSL_PROV_HKDF_CTX *ctx)
 {
-    if (ctx != NULL)
-    {
-        EVP_MD_free(ctx->hkdfCtx->md);
-        scossl_hkdf_freectx(ctx->hkdfCtx);
-    }
+    if (ctx == NULL)
+        return;
 
+    EVP_MD_free(ctx->hkdfCtx->md);
+    scossl_hkdf_freectx(ctx->hkdfCtx);
     OPENSSL_free(ctx);
 }
 
diff --git a/SymCryptProvider/src/kdf/p_scossl_pbkdf2.c b/SymCryptProvider/src/kdf/p_scossl_pbkdf2.c
index c977b44a..dfc1eb89 100644
--- a/SymCryptProvider/src/kdf/p_scossl_pbkdf2.c
+++ b/SymCryptProvider/src/kdf/p_scossl_pbkdf2.c
@@ -78,7 +78,7 @@ SCOSSL_PROV_PBKDF2_CTX *p_scossl_pbkdf2_dupctx(_In_ SCOSSL_PROV_PBKDF2_CTX *ctx)
 {
     SCOSSL_STATUS status = SCOSSL_FAILURE;
 
-    SCOSSL_PROV_PBKDF2_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_PROV_PBKDF2_CTX));
+    SCOSSL_PROV_PBKDF2_CTX *copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_PROV_PBKDF2_CTX));
     if (copyCtx != NULL)
     {
         copyCtx->libctx = ctx->libctx;
@@ -98,17 +98,12 @@ SCOSSL_PROV_PBKDF2_CTX *p_scossl_pbkdf2_dupctx(_In_ SCOSSL_PROV_PBKDF2_CTX *ctx)
             memcpy(copyCtx->pbPassword, ctx->pbPassword, ctx->cbPassword);
             copyCtx->cbPassword = ctx->cbPassword;
         }
-        else
-        {
-            copyCtx->pbPassword = NULL;
-            copyCtx->cbPassword = 0;
-        }
 
-        if ((copyCtx->pbSalt = OPENSSL_memdup(ctx->pbSalt, ctx->cbSalt)) == NULL &&
-            ctx->pbSalt != NULL)
+        if (ctx->pbSalt != NULL &&
+            (copyCtx->pbSalt = OPENSSL_memdup(ctx->pbSalt, ctx->cbSalt)) == NULL)
         {
             ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
-            goto cleanup;            
+            goto cleanup;
         }
         copyCtx->cbSalt = ctx->cbSalt;
     }
diff --git a/SymCryptProvider/src/kdf/p_scossl_srtpkdf.c b/SymCryptProvider/src/kdf/p_scossl_srtpkdf.c
index 19ad3770..1d6866c9 100644
--- a/SymCryptProvider/src/kdf/p_scossl_srtpkdf.c
+++ b/SymCryptProvider/src/kdf/p_scossl_srtpkdf.c
@@ -94,6 +94,9 @@ static SCOSSL_PROV_SRTPKDF_CTX *p_scossl_srtpkdf_dupctx(_In_ SCOSSL_PROV_SRTPKDF
 
     if (copyCtx != NULL)
     {
+        *copyCtx = *ctx;
+        copyCtx->pbKey = NULL;
+
         if (ctx->pbKey != NULL)
         {
             if ((copyCtx->pbKey = OPENSSL_secure_malloc(ctx->cbKey)) == NULL)
@@ -103,7 +106,6 @@ static SCOSSL_PROV_SRTPKDF_CTX *p_scossl_srtpkdf_dupctx(_In_ SCOSSL_PROV_SRTPKDF
             }
 
             memcpy(copyCtx->pbKey, ctx->pbKey, ctx->cbKey);
-            copyCtx->cbKey = ctx->cbKey;
 
             scError = SymCryptSrtpKdfExpandKey(&copyCtx->expandedKey, copyCtx->pbKey, copyCtx->cbKey);
             if (scError != SYMCRYPT_NO_ERROR)
@@ -112,23 +114,6 @@ static SCOSSL_PROV_SRTPKDF_CTX *p_scossl_srtpkdf_dupctx(_In_ SCOSSL_PROV_SRTPKDF
                 goto cleanup;
             }
         }
-        else
-        {
-            copyCtx->pbKey = NULL;
-            copyCtx->cbKey = 0;
-        }
-
-        if (ctx->isSaltSet)
-        {
-            memcpy(copyCtx->pbSalt, ctx->pbSalt, SCOSSL_SRTP_KDF_SALT_SIZE);
-        }
-
-        copyCtx->isSrtcp = ctx->isSrtcp;
-        copyCtx->isSaltSet = ctx->isSaltSet;
-        copyCtx->uKeyDerivationRate = ctx->uKeyDerivationRate;
-        copyCtx->uIndex = ctx->uIndex;
-        copyCtx->uIndexWidth = ctx->uIndexWidth;
-        copyCtx->label = ctx->label;
     }
 
     status = SCOSSL_SUCCESS;
diff --git a/SymCryptProvider/src/kdf/p_scossl_sshkdf.c b/SymCryptProvider/src/kdf/p_scossl_sshkdf.c
index b9b593e0..31a64729 100644
--- a/SymCryptProvider/src/kdf/p_scossl_sshkdf.c
+++ b/SymCryptProvider/src/kdf/p_scossl_sshkdf.c
@@ -62,28 +62,44 @@ SCOSSL_PROV_SSHKDF_CTX *p_scossl_sshkdf_newctx(_In_ SCOSSL_PROVCTX *provctx)
 
 void p_scossl_sshkdf_freectx(_Inout_ SCOSSL_PROV_SSHKDF_CTX *ctx)
 {
-    if (ctx != NULL)
-    {
-        OPENSSL_free(ctx->mdName);
-        scossl_sshkdf_freectx(ctx->sshkdfCtx);
-    }
-
+    if (ctx == NULL)
+        return;
+    
+    OPENSSL_free(ctx->mdName);
+    scossl_sshkdf_freectx(ctx->sshkdfCtx);
     OPENSSL_free(ctx);
 }
 
 SCOSSL_PROV_SSHKDF_CTX *p_scossl_sshkdf_dupctx(_In_ SCOSSL_PROV_SSHKDF_CTX *ctx)
 {
-    SCOSSL_PROV_SSHKDF_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_PROV_SSHKDF_CTX));
+    SCOSSL_STATUS status = SCOSSL_FAILURE;
+
+    SCOSSL_PROV_SSHKDF_CTX *copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_PROV_SSHKDF_CTX));
     if (copyCtx != NULL)
     {
         if ((copyCtx->sshkdfCtx = scossl_sshkdf_dupctx(ctx->sshkdfCtx)) == NULL)
         {
-            OPENSSL_free(copyCtx);
-            return NULL;
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
+        }
+
+        if (ctx->mdName != NULL &&
+            (copyCtx->mdName = OPENSSL_strdup(ctx->mdName)) == NULL)
+        {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
         }
 
         copyCtx->libctx = ctx->libctx;
-        copyCtx->mdName = OPENSSL_strdup(ctx->mdName);
+    }
+
+    status = SCOSSL_SUCCESS;
+
+cleanup:
+    if (status != SCOSSL_SUCCESS)
+    {
+        p_scossl_sshkdf_freectx(copyCtx);
+        copyCtx = NULL;
     }
 
     return copyCtx;
@@ -207,6 +223,12 @@ SCOSSL_STATUS p_scossl_sshkdf_set_ctx_params(_Inout_ SCOSSL_PROV_SSHKDF_CTX *ctx
         symcryptHashAlg = scossl_get_symcrypt_hash_algorithm(EVP_MD_type(md));
         EVP_MD_free(md);
 
+        if (mdName == NULL)
+        {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            return SCOSSL_FAILURE;
+        }
+
         if (symcryptHashAlg == NULL)
         {
             OPENSSL_free(mdName);
diff --git a/SymCryptProvider/src/kdf/p_scossl_tls1prf.c b/SymCryptProvider/src/kdf/p_scossl_tls1prf.c
index 1c9407f9..b8b0d44d 100644
--- a/SymCryptProvider/src/kdf/p_scossl_tls1prf.c
+++ b/SymCryptProvider/src/kdf/p_scossl_tls1prf.c
@@ -44,30 +44,46 @@ SCOSSL_PROV_TLS1_PRF_CTX *p_scossl_tls1prf_newctx(_In_ SCOSSL_PROVCTX *provctx)
 
 void p_scossl_tls1prf_freectx(_Inout_ SCOSSL_PROV_TLS1_PRF_CTX *ctx)
 {
-    if (ctx != NULL)
-    {
-        scossl_tls1prf_freectx(ctx->tls1prfCtx);
-        OPENSSL_free(ctx->mdName);
-    }
+    if (ctx == NULL)
+        return;
 
+    OPENSSL_free(ctx->mdName);
+    scossl_tls1prf_freectx(ctx->tls1prfCtx);
     OPENSSL_free(ctx);
 }
 
 SCOSSL_PROV_TLS1_PRF_CTX *p_scossl_tls1prf_dupctx(_In_ SCOSSL_PROV_TLS1_PRF_CTX *ctx)
 {
-    SCOSSL_PROV_TLS1_PRF_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_PROV_TLS1_PRF_CTX));
+    SCOSSL_STATUS status = SCOSSL_FAILURE;
+
+    SCOSSL_PROV_TLS1_PRF_CTX *copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_PROV_TLS1_PRF_CTX));
     if (copyCtx != NULL)
     {
         if ((copyCtx->tls1prfCtx = scossl_tls1prf_dupctx(ctx->tls1prfCtx)) == NULL)
         {
-            OPENSSL_free(copyCtx);
-            return NULL;
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
+        }
+
+        if (ctx->mdName != NULL &&
+            (copyCtx->mdName = OPENSSL_strdup(ctx->mdName)) == NULL)
+        {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
         }
 
-        copyCtx->mdName = OPENSSL_strdup(ctx->mdName);
         copyCtx->libctx = ctx->libctx;
     }
 
+    status = SCOSSL_SUCCESS;
+
+cleanup:
+    if (status != SCOSSL_SUCCESS)
+    {
+        p_scossl_tls1prf_freectx(copyCtx);
+        copyCtx = NULL;
+    }
+
     return copyCtx;
 }
 
@@ -203,6 +219,11 @@ SCOSSL_STATUS p_scossl_tls1prf_set_ctx_params(_Inout_ SCOSSL_PROV_TLS1_PRF_CTX *
         OPENSSL_free(ctx->mdName);
         ctx->mdName = mdName;
         mdName = NULL;
+        if (ctx->mdName == NULL)
+        {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
+        }
 
         ctx->tls1prfCtx->pHmac = symcryptHmacAlg;
         ctx->tls1prfCtx->isTlsPrf1_1 = isTlsPrf1_1;
diff --git a/SymCryptProvider/src/kem/p_scossl_mlkem.c b/SymCryptProvider/src/kem/p_scossl_mlkem.c
index ff06100a..cabe133a 100644
--- a/SymCryptProvider/src/kem/p_scossl_mlkem.c
+++ b/SymCryptProvider/src/kem/p_scossl_mlkem.c
@@ -68,12 +68,10 @@ static SCOSSL_MLKEM_CTX *p_scossl_mlkem_dupctx(_In_ SCOSSL_MLKEM_CTX *ctx)
         copyCtx->keyCtx = ctx->keyCtx;
         copyCtx->operation = ctx->operation;
         copyCtx->provCtx = ctx->provCtx;
+        copyCtx->classicKeyexchCtx = NULL;
 
-        if (ctx->classicKeyexchCtx != NULL)
-        {
-            copyCtx->classicKeyexchCtx = NULL;
-        }
-        else if ((copyCtx->classicKeyexchCtx = p_scossl_ecdh_dupctx(ctx->classicKeyexchCtx)) == NULL)
+        if (ctx->classicKeyexchCtx != NULL &&
+            (copyCtx->classicKeyexchCtx = p_scossl_ecdh_dupctx(ctx->classicKeyexchCtx)) == NULL)
         {
             OPENSSL_free(copyCtx);
             copyCtx = NULL;
diff --git a/SymCryptProvider/src/keyexch/p_scossl_dh.c b/SymCryptProvider/src/keyexch/p_scossl_dh.c
index 1aaf7cd4..402be1a6 100644
--- a/SymCryptProvider/src/keyexch/p_scossl_dh.c
+++ b/SymCryptProvider/src/keyexch/p_scossl_dh.c
@@ -97,10 +97,10 @@ static SCOSSL_DH_CTX *p_scossl_dh_dupctx(_In_ SCOSSL_DH_CTX *ctx)
         copyCtx->kdfCekAlg = OPENSSL_strdup(ctx->kdfCekAlg);
         copyCtx->kdfUkm = OPENSSL_memdup(ctx->kdfUkm, ctx->kdfUkmlen);
 
-        if ((ctx->kdfMdName != NULL && (copyCtx->kdfMdName == NULL)) ||
+        if ((ctx->kdfMdName != NULL  && (copyCtx->kdfMdName == NULL)) ||
             (ctx->kdfMdProps != NULL && (copyCtx->kdfMdProps == NULL)) ||
-            (ctx->kdfCekAlg != NULL && (copyCtx->kdfCekAlg == NULL)) ||
-            (ctx->kdfUkm != NULL && (copyCtx->kdfUkm == NULL)))
+            (ctx->kdfCekAlg != NULL  && (copyCtx->kdfCekAlg == NULL)) ||
+            (ctx->kdfUkm != NULL     && (copyCtx->kdfUkm == NULL)))
         {
             p_scossl_dh_freectx(copyCtx);
             copyCtx = NULL;
diff --git a/SymCryptProvider/src/keyexch/p_scossl_ecdh.c b/SymCryptProvider/src/keyexch/p_scossl_ecdh.c
index 1139eebb..303ad63d 100644
--- a/SymCryptProvider/src/keyexch/p_scossl_ecdh.c
+++ b/SymCryptProvider/src/keyexch/p_scossl_ecdh.c
@@ -43,9 +43,7 @@ SCOSSL_ECDH_CTX *p_scossl_ecdh_dupctx(SCOSSL_ECDH_CTX *ctx)
     SCOSSL_ECDH_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_ECDH_CTX));
     if (copyCtx != NULL)
     {
-        copyCtx->libctx = ctx->libctx;
-        copyCtx->keyCtx = ctx->keyCtx;
-        copyCtx->peerKeyCtx = ctx->peerKeyCtx;
+        *copyCtx = *ctx;
     }
 
     return copyCtx;
diff --git a/SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c b/SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c
index 00675a0d..b0dacb33 100644
--- a/SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c
+++ b/SymCryptProvider/src/keymgmt/p_scossl_dh_keymgmt.c
@@ -12,7 +12,7 @@
 extern "C" {
 #endif
 
-#define SCOSSL_DH_KEYHEN_POSSIBLE_SELECTIONS (OSSL_KEYMGMT_SELECT_KEYPAIR | OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS)
+#define SCOSSL_DH_KEYGEN_POSSIBLE_SELECTIONS (OSSL_KEYMGMT_SELECT_KEYPAIR | OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS)
 
 #define SCOSSL_DH_PBITS_DEFAULT 2048
 // Private key length determined by group
@@ -105,15 +105,13 @@ static SCOSSL_PROV_DH_KEY_CTX *p_scossl_dh_keymgmt_new_ctx(_In_ SCOSSL_PROVCTX *
         if ((ctx->keyCtx = scossl_dh_new_key_ctx()) == NULL)
         {
             OPENSSL_free(ctx);
-            ctx = NULL;
-        }
-        else
-        {
-            ctx->pDlGroup = NULL;
-            ctx->groupSetByParams = FALSE;
-            ctx->nBitsPriv = SCOSSL_DH_PRIVATE_BITS_DEFAULT;
-            ctx->libCtx = provCtx->libctx;
+            return NULL;
         }
+
+        ctx->pDlGroup = NULL;
+        ctx->groupSetByParams = FALSE;
+        ctx->nBitsPriv = SCOSSL_DH_PRIVATE_BITS_DEFAULT;
+        ctx->libCtx = provCtx->libctx;
     }
 
     return ctx;
@@ -500,7 +498,7 @@ static SCOSSL_DH_KEYGEN_CTX *p_scossl_dh_keygen_init(_In_ SCOSSL_PROVCTX *provCt
 {
     SCOSSL_DH_KEYGEN_CTX *genCtx = NULL;
 
-    if ((selection & SCOSSL_DH_KEYHEN_POSSIBLE_SELECTIONS) != 0 &&
+    if ((selection & SCOSSL_DH_KEYGEN_POSSIBLE_SELECTIONS) != 0 &&
         (genCtx = OPENSSL_malloc(sizeof(SCOSSL_DH_KEYGEN_CTX))) != NULL)
     {
         genCtx->pDlGroup = NULL;
diff --git a/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c b/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
index 04d2ca6b..fe3bac4d 100644
--- a/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
+++ b/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
@@ -226,7 +226,7 @@ static SCOSSL_STATUS p_scossl_rsa_keymgmt_dup_keydata(_In_ PCSYMCRYPT_RSAKEY fro
 
 static SCOSSL_PROV_RSA_KEY_CTX *p_scossl_rsa_keymgmt_dup_ctx(_In_ const SCOSSL_PROV_RSA_KEY_CTX *keyCtx, int selection)
 {
-    SCOSSL_PROV_RSA_KEY_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_PROV_RSA_KEY_CTX));
+    SCOSSL_PROV_RSA_KEY_CTX *copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_PROV_RSA_KEY_CTX));
     if (copyCtx == NULL)
     {
         return NULL;
diff --git a/SymCryptProvider/src/mac/p_scossl_hmac.c b/SymCryptProvider/src/mac/p_scossl_hmac.c
index eb766413..364d245d 100644
--- a/SymCryptProvider/src/mac/p_scossl_hmac.c
+++ b/SymCryptProvider/src/mac/p_scossl_hmac.c
@@ -181,7 +181,11 @@ static SCOSSL_STATUS p_scossl_hmac_set_ctx_params(_Inout_ SCOSSL_MAC_CTX *ctx, _
             goto cleanup;
         }
 
-        ctx->mdName = OPENSSL_strdup(mdName);
+        if ((ctx->mdName = OPENSSL_strdup(mdName)) == NULL)
+        {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
+        }
     }
 
     if ((p = OSSL_PARAM_locate_const(params, OSSL_MAC_PARAM_KEY)) != NULL)
diff --git a/SymCryptProvider/src/p_scossl_ecc.c b/SymCryptProvider/src/p_scossl_ecc.c
index 5b0844e3..59c1a327 100644
--- a/SymCryptProvider/src/p_scossl_ecc.c
+++ b/SymCryptProvider/src/p_scossl_ecc.c
@@ -55,7 +55,7 @@ SCOSSL_ECC_KEY_CTX *p_scossl_ecc_dup_ctx(SCOSSL_ECC_KEY_CTX *keyCtx, int selecti
     SYMCRYPT_ECPOINT_FORMAT pointFormat = keyCtx->isX25519 ? SYMCRYPT_ECPOINT_FORMAT_X : SYMCRYPT_ECPOINT_FORMAT_XY;
     SYMCRYPT_ERROR scError = SYMCRYPT_NO_ERROR;
 
-    SCOSSL_ECC_KEY_CTX *copyCtx = OPENSSL_malloc(sizeof(SCOSSL_ECC_KEY_CTX));
+    SCOSSL_ECC_KEY_CTX *copyCtx = OPENSSL_zalloc(sizeof(SCOSSL_ECC_KEY_CTX));
 
     if (copyCtx != NULL)
     {

From a69991efb7cb07ba7f799bbff72696e35d58b343 Mon Sep 17 00:00:00 2001
From: Samuel Lee <56448320+samuel-lee-msft@users.noreply.github.com>
Date: Thu, 17 Jul 2025 13:38:09 -0700
Subject: [PATCH 4/4] Additional allocation hardening (#137)

* Based on additional fixes made when porting previous fix to 1.9 branch
  (#135)
* Check for failure in CRYPTO_THREAD_lock_new
* Reduce scope for unexpected behavior in RSA keygen
---
 KeysInUse/keysinuse.c                               |  4 ++++
 ScosslCommon/src/scossl_helpers.c                   | 10 ++++++++++
 SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c |  5 +++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/KeysInUse/keysinuse.c b/KeysInUse/keysinuse.c
index 91481f92..47f98e31 100644
--- a/KeysInUse/keysinuse.c
+++ b/KeysInUse/keysinuse.c
@@ -241,6 +241,10 @@ static void keysinuse_init_internal()
 
     lh_keysinuse_ctx_imp_lock = CRYPTO_THREAD_lock_new();
     lh_keysinuse_ctx_imp = lh_SCOSSL_KEYSINUSE_CTX_IMP_new(scossl_keysinuse_ctx_hash, scossl_keysinuse_ctx_cmp);
+    if (lh_keysinuse_ctx_imp_lock == NULL || lh_keysinuse_ctx_imp == NULL)
+    {
+        goto cleanup;
+    }
 
 #ifndef KEYSINUSE_LOG_SYSLOG
     if (!keysinuse_init_logging())
diff --git a/ScosslCommon/src/scossl_helpers.c b/ScosslCommon/src/scossl_helpers.c
index d61def33..8ada248b 100644
--- a/ScosslCommon/src/scossl_helpers.c
+++ b/ScosslCommon/src/scossl_helpers.c
@@ -198,6 +198,11 @@ void SCOSSL_set_trace_level(int trace_level, int ossl_ERR_level)
 
 void SCOSSL_set_trace_log_filename(const char *filename)
 {
+    if( _loggingLock == NULL )
+    {
+        return;
+    }
+
     if( _traceLogFilename )
     {
         OPENSSL_free(_traceLogFilename);
@@ -239,6 +244,11 @@ static void _scossl_log_bytes_valist(
     char errStringBuf[SCOSSL_TRACELOG_PARA_LENGTH];
     char paraBuf[SCOSSL_TRACELOG_PARA_LENGTH];
     char *trace_level_prefix = "";
+    
+    if( _loggingLock == NULL )
+    {
+        return;
+    }
 
     if( SYMCRYPT_MAX(_traceLogLevel, _osslERRLogLevel) < trace_level )
     {
diff --git a/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c b/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
index fe3bac4d..0cdace31 100644
--- a/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
+++ b/SymCryptProvider/src/keymgmt/p_scossl_rsa_keymgmt.c
@@ -401,7 +401,7 @@ static SCOSSL_PROV_RSA_KEY_CTX *p_scossl_rsa_keygen(_In_ SCOSSL_RSA_KEYGEN_CTX *
     PUINT64 pPubExp64;
     UINT32 genFlags = SYMCRYPT_FLAG_RSAKEY_SIGN | SYMCRYPT_FLAG_RSAKEY_ENCRYPT;
 
-    keyCtx = OPENSSL_malloc(sizeof(SCOSSL_PROV_RSA_KEY_CTX));
+    keyCtx = OPENSSL_zalloc(sizeof(SCOSSL_PROV_RSA_KEY_CTX));
     if (keyCtx == NULL)
     {
         goto cleanup;
@@ -434,7 +434,6 @@ static SCOSSL_PROV_RSA_KEY_CTX *p_scossl_rsa_keygen(_In_ SCOSSL_RSA_KEYGEN_CTX *
         goto cleanup;
     }
 
-    keyCtx->initialized = TRUE;
     keyCtx->keyType = genCtx->keyType;
     keyCtx->pssRestrictions = genCtx->pssRestrictions;
     genCtx->pssRestrictions = NULL;
@@ -443,6 +442,8 @@ static SCOSSL_PROV_RSA_KEY_CTX *p_scossl_rsa_keygen(_In_ SCOSSL_RSA_KEYGEN_CTX *
     keyCtx->keysinuseCtx = NULL;
 #endif
 
+    keyCtx->initialized = TRUE;
+
 cleanup:
     if (keyCtx != NULL && !keyCtx->initialized)
     {