Bug 1576307 - Check mechanism param and param length before casting to mechanism-specific structs. r=jcj
authorKevin Jacobs <kjacobs@mozilla.com>
Mon, 30 Sep 2019 22:16:21 +0000
changeset 15317 53d92a32408049038f450aa747b0030607988230
parent 15316 c0913ad7a5609751a8dfc37ae2e0a7a0cd6a42dd
child 15326 dc86215aea17672228b28cf3516da89a751cc24d
push id3522
push userjjones@mozilla.com
push dateWed, 02 Oct 2019 20:34:11 +0000
reviewersjcj
bugs1576307
Bug 1576307 - Check mechanism param and param length before casting to mechanism-specific structs. r=jcj This patch adds missing PKCS11 input parameter checks, which are needed prior to casting to mechanism-specific structs. Differential Revision: https://phabricator.services.mozilla.com/D44075
gtests/pk11_gtest/pk11_cbc_unittest.cc
lib/softoken/pkcs11c.c
--- a/gtests/pk11_gtest/pk11_cbc_unittest.cc
+++ b/gtests/pk11_gtest/pk11_cbc_unittest.cc
@@ -228,16 +228,100 @@ TEST_P(Pkcs11CbcPadTest, FailEncryptSimp
 
   SECStatus rv =
       PK11_Encrypt(ek.get(), GetParam(), GetIv(), output, &output_len,
                    sizeof(output), kInput, GetInputLen(CKA_ENCRYPT));
   EXPECT_EQ(SECFailure, rv);
   EXPECT_EQ(333U, output_len);
 }
 
+// It's a bit of a lie to put this in pk11_cbc_unittest, since we
+// also test bounds checking in other modes. There doesn't seem
+// to be an appropriately-generic place elsewhere.
+TEST_F(Pkcs11CbcPadTest, FailEncryptShortParam) {
+  SECStatus rv = SECFailure;
+  uint8_t encrypted[sizeof(kInput)];
+  unsigned int encrypted_len = 0;
+  size_t input_len = AES_BLOCK_SIZE;
+
+  // CK_GCM_PARAMS is the largest param struct used across AES modes
+  uint8_t param_buf[sizeof(CK_GCM_PARAMS)];
+  SECItem param = {siBuffer, param_buf, sizeof(param_buf)};
+  SECItem key_item = {siBuffer, const_cast<uint8_t*>(kKeyData), 16};
+
+  // Setup (we use the ECB key for other modes)
+  ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
+  ASSERT_NE(nullptr, slot);
+  ScopedPK11SymKey key(PK11_ImportSymKey(slot.get(), CKM_AES_ECB,
+                                         PK11_OriginUnwrap, CKA_ENCRYPT,
+                                         &key_item, nullptr));
+  ASSERT_TRUE(key.get());
+
+  // CTR should have a CK_AES_CTR_PARAMS
+  param.len = sizeof(CK_AES_CTR_PARAMS) - 1;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CTR, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECFailure, rv);
+
+  param.len++;
+  reinterpret_cast<CK_AES_CTR_PARAMS*>(param.data)->ulCounterBits = 32;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CTR, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECSuccess, rv);
+
+  // GCM should have a CK_GCM_PARAMS
+  param.len = sizeof(CK_GCM_PARAMS) - 1;
+  rv = PK11_Encrypt(key.get(), CKM_AES_GCM, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECFailure, rv);
+
+  param.len++;
+  reinterpret_cast<CK_GCM_PARAMS*>(param.data)->pIv = param_buf;
+  reinterpret_cast<CK_GCM_PARAMS*>(param.data)->ulIvLen = 12;
+  reinterpret_cast<CK_GCM_PARAMS*>(param.data)->pAAD = nullptr;
+  reinterpret_cast<CK_GCM_PARAMS*>(param.data)->ulAADLen = 0;
+  reinterpret_cast<CK_GCM_PARAMS*>(param.data)->ulTagBits = 128;
+  rv = PK11_Encrypt(key.get(), CKM_AES_GCM, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECSuccess, rv);
+
+  // CBC (and the below modes) should have a 16B IV
+  param.len = AES_BLOCK_SIZE - 1;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECFailure, rv);
+
+  param.len++;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECSuccess, rv);
+
+  // ECB
+  param.len = AES_BLOCK_SIZE - 1;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECFailure, rv);
+
+  param.len++;
+  rv = PK11_Encrypt(key.get(), CKM_AES_ECB, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECSuccess, rv);
+
+  // CTS
+  param.len = AES_BLOCK_SIZE - 1;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECFailure, rv);
+
+  param.len++;
+  rv = PK11_Encrypt(key.get(), CKM_AES_CTS, &param, encrypted, &encrypted_len,
+                    sizeof(encrypted), kInput, input_len);
+  EXPECT_EQ(SECSuccess, rv);
+}
+
 TEST_P(Pkcs11CbcPadTest, ContextFailDecryptSimple) {
   ScopedPK11Context dctx = MakeContext(CKA_DECRYPT);
   uint8_t output[sizeof(kInput) + 64];
   int output_len = 77;
 
   SECStatus rv = PK11_CipherOp(dctx.get(), output, &output_len, sizeof(output),
                                kInput, GetInputLen(CKA_DECRYPT));
   EXPECT_EQ(SECSuccess, rv);
--- a/lib/softoken/pkcs11c.c
+++ b/lib/softoken/pkcs11c.c
@@ -35,17 +35,17 @@
 #include "softoken.h"
 #include "secasn1.h"
 #include "secerr.h"
 
 #include "prprf.h"
 #include "prenv.h"
 
 #define __PASTE(x, y) x##y
-
+#define BAD_PARAM_CAST(pMech, typeSize) (!pMech->pParameter || pMech->ulParameterLen < typeSize)
 /*
  * we renamed all our internal functions, get the correct
  * definitions for them...
  */
 #undef CK_PKCS11_FUNCTION_INFO
 #undef CK_NEED_ARG_LIST
 
 #define CK_EXTERN extern
@@ -778,16 +778,20 @@ sftk_CryptInit(CK_SESSION_HANDLE hSessio
 #endif
     CK_KEY_TYPE key_type;
     CK_RV crv = CKR_OK;
     unsigned effectiveKeyLength;
     unsigned char newdeskey[24];
     PRBool useNewKey = PR_FALSE;
     int t;
 
+    if (!pMechanism) {
+        return CKR_MECHANISM_PARAM_INVALID;
+    }
+
     crv = sftk_MechAllowsOperation(pMechanism->mechanism, mechUsage);
     if (crv != CKR_OK)
         return crv;
 
     session = sftk_SessionFromHandle(hSession);
     if (session == NULL)
         return CKR_SESSION_HANDLE_INVALID;
 
@@ -891,16 +895,21 @@ sftk_CryptInit(CK_SESSION_HANDLE hSessio
                 crv = CKR_KEY_TYPE_INCONSISTENT;
                 break;
             }
             att = sftk_FindAttribute(key, CKA_VALUE);
             if (att == NULL) {
                 crv = CKR_KEY_HANDLE_INVALID;
                 break;
             }
+
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_RC2_CBC_PARAMS))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             rc2_param = (CK_RC2_CBC_PARAMS *)pMechanism->pParameter;
             effectiveKeyLength = (rc2_param->ulEffectiveBits + 7) / 8;
             context->cipherInfo =
                 RC2_CreateContext((unsigned char *)att->attrib.pValue,
                                   att->attrib.ulValueLen, rc2_param->iv,
                                   pMechanism->mechanism == CKM_RC2_ECB ? NSS_RC2 : NSS_RC2_CBC, effectiveKeyLength);
             sftk_FreeAttribute(att);
             if (context->cipherInfo == NULL) {
@@ -920,16 +929,21 @@ sftk_CryptInit(CK_SESSION_HANDLE hSessio
                 crv = CKR_KEY_TYPE_INCONSISTENT;
                 break;
             }
             att = sftk_FindAttribute(key, CKA_VALUE);
             if (att == NULL) {
                 crv = CKR_KEY_HANDLE_INVALID;
                 break;
             }
+
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_RC5_CBC_PARAMS))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             rc5_param = (CK_RC5_CBC_PARAMS *)pMechanism->pParameter;
             context->blockSize = rc5_param->ulWordsize * 2;
             rc5Key.data = (unsigned char *)att->attrib.pValue;
             rc5Key.len = att->attrib.ulValueLen;
             context->cipherInfo = RC5_CreateContext(&rc5Key, rc5_param->ulRounds,
                                                     rc5_param->ulWordsize, rc5_param->pIv,
                                                     pMechanism->mechanism == CKM_RC5_ECB ? NSS_RC5 : NSS_RC5_CBC);
             sftk_FreeAttribute(att);
@@ -1117,16 +1131,24 @@ sftk_CryptInit(CK_SESSION_HANDLE hSessio
             context->doPad = PR_TRUE;
         /* fall thru */
         case CKM_AES_ECB:
         case CKM_AES_CBC:
             context->blockSize = 16;
         case CKM_AES_CTS:
         case CKM_AES_CTR:
         case CKM_AES_GCM:
+            /* Note the catch-all only applies to the above cases */
+            if ((pMechanism->mechanism == CKM_AES_GCM && BAD_PARAM_CAST(pMechanism, sizeof(CK_GCM_PARAMS))) ||
+                (pMechanism->mechanism == CKM_AES_CTR && BAD_PARAM_CAST(pMechanism, sizeof(CK_AES_CTR_PARAMS))) ||
+                BAD_PARAM_CAST(pMechanism, AES_BLOCK_SIZE) /* Cast target is an IV */) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
+
             if (pMechanism->mechanism == CKM_AES_GCM) {
                 context->multi = PR_FALSE;
             }
             if (key_type != CKK_AES) {
                 crv = CKR_KEY_TYPE_INCONSISTENT;
                 break;
             }
             att = sftk_FindAttribute(key, CKA_VALUE);
@@ -2211,19 +2233,23 @@ sftk_InitCBCMac(CK_SESSION_HANDLE hSessi
     unsigned char ivBlock[SFTK_MAX_BLOCK_SIZE];
     unsigned char k2[SFTK_MAX_BLOCK_SIZE];
     unsigned char k3[SFTK_MAX_BLOCK_SIZE];
     SFTKSessionContext *context;
     CK_RV crv;
     unsigned int blockSize;
     PRBool isXCBC = PR_FALSE;
 
+    if (!pMechanism) {
+        return CKR_MECHANISM_PARAM_INVALID;
+    }
+
     switch (pMechanism->mechanism) {
         case CKM_RC2_MAC_GENERAL:
-            if (!pMechanism->pParameter) {
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_RC2_MAC_GENERAL_PARAMS))) {
                 return CKR_MECHANISM_PARAM_INVALID;
             }
             mac_bytes =
                 ((CK_RC2_MAC_GENERAL_PARAMS *)pMechanism->pParameter)->ulMacLength;
         /* fall through */
         case CKM_RC2_MAC:
             /* this works because ulEffectiveBits is in the same place in both the
              * CK_RC2_MAC_GENERAL_PARAMS and CK_RC2_CBC_PARAMS */
@@ -2233,22 +2259,28 @@ sftk_InitCBCMac(CK_SESSION_HANDLE hSessi
             PORT_Memset(rc2_params.iv, 0, sizeof(rc2_params.iv));
             cbc_mechanism.mechanism = CKM_RC2_CBC;
             cbc_mechanism.pParameter = &rc2_params;
             cbc_mechanism.ulParameterLen = sizeof(rc2_params);
             blockSize = 8;
             break;
 #if NSS_SOFTOKEN_DOES_RC5
         case CKM_RC5_MAC_GENERAL:
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_RC5_MAC_GENERAL_PARAMS))) {
+                return CKR_MECHANISM_PARAM_INVALID;
+            }
             mac_bytes =
                 ((CK_RC5_MAC_GENERAL_PARAMS *)pMechanism->pParameter)->ulMacLength;
         /* fall through */
         case CKM_RC5_MAC:
             /* this works because ulEffectiveBits is in the same place in both the
              * CK_RC5_MAC_GENERAL_PARAMS and CK_RC5_CBC_PARAMS */
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_RC5_MAC_GENERAL_PARAMS))) {
+                return CKR_MECHANISM_PARAM_INVALID;
+            }
             rc5_mac = (CK_RC5_MAC_GENERAL_PARAMS *)pMechanism->pParameter;
             rc5_params.ulWordsize = rc5_mac->ulWordsize;
             rc5_params.ulRounds = rc5_mac->ulRounds;
             rc5_params.pIv = ivBlock;
             if ((blockSize = rc5_mac->ulWordsize * 2) > SFTK_MAX_BLOCK_SIZE)
                 return CKR_MECHANISM_PARAM_INVALID;
             rc5_params.ulIvLen = blockSize;
             PORT_Memset(ivBlock, 0, blockSize);
@@ -3842,21 +3874,27 @@ nsc_pbe_key_gen(NSSPKCS5PBEParameter *pk
     CK_PBE_PARAMS *pbe_params = NULL;
     CK_PKCS5_PBKD2_PARAMS *pbkd2_params = NULL;
 
     *key_length = 0;
     iv.data = NULL;
     iv.len = 0;
 
     if (pMechanism->mechanism == CKM_PKCS5_PBKD2) {
+        if (BAD_PARAM_CAST(pMechanism, sizeof(CK_PKCS5_PBKD2_PARAMS))) {
+            return CKR_MECHANISM_PARAM_INVALID;
+        }
         pbkd2_params = (CK_PKCS5_PBKD2_PARAMS *)pMechanism->pParameter;
         pwitem.data = (unsigned char *)pbkd2_params->pPassword;
         /* was this a typo in the PKCS #11 spec? */
         pwitem.len = *pbkd2_params->ulPasswordLen;
     } else {
+        if (BAD_PARAM_CAST(pMechanism, sizeof(CK_PBE_PARAMS))) {
+            return CKR_MECHANISM_PARAM_INVALID;
+        }
         pbe_params = (CK_PBE_PARAMS *)pMechanism->pParameter;
         pwitem.data = (unsigned char *)pbe_params->pPassword;
         pwitem.len = pbe_params->ulPasswordLen;
     }
     pbe_key = nsspkcs5_ComputeKeyAndIV(pkcs5_pbe, &pwitem, &iv, faulty3DES);
     if (pbe_key == NULL) {
         return CKR_HOST_MEMORY;
     }
@@ -4146,16 +4184,20 @@ nsc_SetupHMACKeyGen(CK_MECHANISM_PTR pMe
     }
 
     params = (NSSPKCS5PBEParameter *)PORT_ArenaZAlloc(arena,
                                                       sizeof(NSSPKCS5PBEParameter));
     if (params == NULL) {
         PORT_FreeArena(arena, PR_TRUE);
         return CKR_HOST_MEMORY;
     }
+    if (BAD_PARAM_CAST(pMechanism, sizeof(CK_PBE_PARAMS))) {
+        PORT_FreeArena(arena, PR_TRUE);
+        return CKR_MECHANISM_PARAM_INVALID;
+    }
 
     params->poolp = arena;
     params->ivLen = 0;
     params->pbeType = NSSPKCS5_PKCS12_V2;
     params->hashType = HASH_AlgSHA1;
     params->encAlg = SEC_OID_SHA1; /* any invalid value */
     params->is2KeyDES = PR_FALSE;
     params->keyID = pbeBitGenIntegrityKey;
@@ -4225,20 +4267,20 @@ nsc_SetupPBEKeyGen(CK_MECHANISM_PTR pMec
     *pbe = NULL;
 
     oid = SECOID_FindOIDByMechanism(pMechanism->mechanism);
     if (oid == NULL) {
         return CKR_MECHANISM_INVALID;
     }
 
     if (pMechanism->mechanism == CKM_PKCS5_PBKD2) {
-        pbkd2_params = (CK_PKCS5_PBKD2_PARAMS *)pMechanism->pParameter;
-        if (pbkd2_params == NULL) {
+        if (BAD_PARAM_CAST(pMechanism, sizeof(CK_PKCS5_PBKD2_PARAMS))) {
             return CKR_MECHANISM_PARAM_INVALID;
         }
+        pbkd2_params = (CK_PKCS5_PBKD2_PARAMS *)pMechanism->pParameter;
         switch (pbkd2_params->prf) {
             case CKP_PKCS5_PBKD2_HMAC_SHA1:
                 hashType = HASH_AlgSHA1;
                 break;
             case CKP_PKCS5_PBKD2_HMAC_SHA224:
                 hashType = HASH_AlgSHA224;
                 break;
             case CKP_PKCS5_PBKD2_HMAC_SHA256:
@@ -4255,16 +4297,19 @@ nsc_SetupPBEKeyGen(CK_MECHANISM_PTR pMec
         }
         if (pbkd2_params->saltSource != CKZ_SALT_SPECIFIED) {
             return CKR_MECHANISM_PARAM_INVALID;
         }
         salt.data = (unsigned char *)pbkd2_params->pSaltSourceData;
         salt.len = (unsigned int)pbkd2_params->ulSaltSourceDataLen;
         iteration = pbkd2_params->iterations;
     } else {
+        if (BAD_PARAM_CAST(pMechanism, sizeof(CK_PBE_PARAMS))) {
+            return CKR_MECHANISM_PARAM_INVALID;
+        }
         pbe_params = (CK_PBE_PARAMS *)pMechanism->pParameter;
         salt.data = (unsigned char *)pbe_params->pSalt;
         salt.len = (unsigned int)pbe_params->ulSaltLen;
         iteration = pbe_params->ulIteration;
     }
     params = nsspkcs5_NewParam(oid->offset, hashType, &salt, iteration);
     if (params == NULL) {
         return CKR_MECHANISM_INVALID;
@@ -4374,18 +4419,17 @@ NSC_GenerateKey(CK_SESSION_HANDLE hSessi
             continue;
         }
 
         crv = sftk_AddAttributeType(key, sftk_attr_expand(&pTemplate[i]));
         if (crv != CKR_OK)
             break;
     }
     if (crv != CKR_OK) {
-        sftk_FreeObject(key);
-        return crv;
+        goto loser;
     }
 
     /* make sure we don't have any class, key_type, or value fields */
     sftk_DeleteAttributeType(key, CKA_CLASS);
     sftk_DeleteAttributeType(key, CKA_KEY_TYPE);
     sftk_DeleteAttributeType(key, CKA_VALUE);
 
     /* Now Set up the parameters to generate the key (based on mechanism) */
@@ -4488,18 +4532,17 @@ NSC_GenerateKey(CK_SESSION_HANDLE hSessi
     /* make sure we aren't going to overflow the buffer */
     if (sizeof(buf) < key_length) {
         /* someone is getting pretty optimistic about how big their key can
          * be... */
         crv = CKR_TEMPLATE_INCONSISTENT;
     }
 
     if (crv != CKR_OK) {
-        sftk_FreeObject(key);
-        return crv;
+        goto loser;
     }
 
     /* if there was no error,
      * key_type *MUST* be set in the switch statement above */
     PORT_Assert(key_type != CKK_INVALID_KEY_TYPE);
 
     /*
      * now to the actual key gen.
@@ -4507,16 +4550,20 @@ NSC_GenerateKey(CK_SESSION_HANDLE hSessi
     switch (key_gen_type) {
         case nsc_pbe:
             crv = nsc_pbe_key_gen(pbe_param, pMechanism, buf, &key_length,
                                   faultyPBE3DES);
             nsspkcs5_DestroyPBEParameter(pbe_param);
             break;
         case nsc_ssl:
             rsa_pms = (SSL3RSAPreMasterSecret *)buf;
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_VERSION))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                goto loser;
+            }
             version = (CK_VERSION *)pMechanism->pParameter;
             rsa_pms->client_version[0] = version->major;
             rsa_pms->client_version[1] = version->minor;
             crv =
                 NSC_GenerateRandom(0, &rsa_pms->random[0], sizeof(rsa_pms->random));
             break;
         case nsc_bulk:
             /* get the key, check for weak keys and repeat if found */
@@ -4525,67 +4572,68 @@ NSC_GenerateKey(CK_SESSION_HANDLE hSessi
             } while (crv == CKR_OK && checkWeak && sftk_IsWeakKey(buf, key_type));
             break;
         case nsc_param:
             /* generate parameters */
             *buf = 0;
             crv = nsc_parameter_gen(key_type, key);
             break;
         case nsc_jpake:
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_NSS_JPAKERound1Params))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                goto loser;
+            }
             crv = jpake_Round1(hashType,
                                (CK_NSS_JPAKERound1Params *)pMechanism->pParameter,
                                key);
             break;
     }
 
     if (crv != CKR_OK) {
-        sftk_FreeObject(key);
-        return crv;
+        goto loser;
     }
 
     /* Add the class, key_type, and value */
     crv = sftk_AddAttributeType(key, CKA_CLASS, &objclass, sizeof(CK_OBJECT_CLASS));
     if (crv != CKR_OK) {
-        sftk_FreeObject(key);
-        return crv;
+        goto loser;
     }
     crv = sftk_AddAttributeType(key, CKA_KEY_TYPE, &key_type, sizeof(CK_KEY_TYPE));
     if (crv != CKR_OK) {
-        sftk_FreeObject(key);
-        return crv;
+        goto loser;
     }
     if (key_length != 0) {
         crv = sftk_AddAttributeType(key, CKA_VALUE, buf, key_length);
         if (crv != CKR_OK) {
-            sftk_FreeObject(key);
-            return crv;
+            goto loser;
         }
     }
 
     /* get the session */
     session = sftk_SessionFromHandle(hSession);
     if (session == NULL) {
-        sftk_FreeObject(key);
-        return CKR_SESSION_HANDLE_INVALID;
+        crv = CKR_SESSION_HANDLE_INVALID;
+        goto loser;
     }
 
     /*
      * handle the base object stuff
      */
     crv = sftk_handleObject(key, session);
     sftk_FreeSession(session);
     if (crv == CKR_OK && sftk_isTrue(key, CKA_SENSITIVE)) {
         crv = sftk_forceAttribute(key, CKA_ALWAYS_SENSITIVE, &cktrue, sizeof(CK_BBOOL));
     }
     if (crv == CKR_OK && !sftk_isTrue(key, CKA_EXTRACTABLE)) {
         crv = sftk_forceAttribute(key, CKA_NEVER_EXTRACTABLE, &cktrue, sizeof(CK_BBOOL));
     }
     if (crv == CKR_OK) {
         *phKey = key->handle;
     }
+loser:
     sftk_FreeObject(key);
     return crv;
 }
 
 #define PAIRWISE_DIGEST_LENGTH SHA1_LENGTH /* 160-bits */
 #define PAIRWISE_MESSAGE_LENGTH 20         /* 160-bits */
 
 /*
@@ -6605,17 +6653,16 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
     CK_ULONG tmpKeySize;
     CK_ULONG IVSize;
     CK_ULONG keySize = 0;
     CK_RV crv = CKR_OK;
     CK_BBOOL cktrue = CK_TRUE;
     CK_KEY_TYPE keyType = CKK_GENERIC_SECRET;
     CK_OBJECT_CLASS classType = CKO_SECRET_KEY;
     CK_KEY_DERIVATION_STRING_DATA *stringPtr;
-    CK_MECHANISM_TYPE mechanism = pMechanism->mechanism;
     PRBool isTLS = PR_FALSE;
     PRBool isDH = PR_FALSE;
     HASH_HashType tlsPrfHash = HASH_AlgNULL;
     SECStatus rv;
     int i;
     unsigned int outLen;
     unsigned char sha_out[SHA1_LENGTH];
     unsigned char key_block[NUM_MIXERS * SFTK_MAX_MAC_LENGTH];
@@ -6623,16 +6670,21 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
     HASH_HashType hashType;
     PRBool extractValue = PR_TRUE;
 
     CHECK_FORK();
 
     if (!slot) {
         return CKR_SESSION_HANDLE_INVALID;
     }
+    if (!pMechanism) {
+        return CKR_MECHANISM_PARAM_INVALID;
+    }
+    CK_MECHANISM_TYPE mechanism = pMechanism->mechanism;
+
     /*
      * now lets create an object to hang the attributes off of
      */
     if (phKey)
         *phKey = CK_INVALID_HANDLE;
 
     key = sftk_NewObject(slot); /* fill in the handle later */
     if (key == NULL) {
@@ -6796,16 +6848,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
         case CKM_SSL3_MASTER_KEY_DERIVE:
         case CKM_SSL3_MASTER_KEY_DERIVE_DH: {
             CK_SSL3_MASTER_KEY_DERIVE_PARAMS *ssl3_master;
             SSL3RSAPreMasterSecret *rsa_pms;
             unsigned char crsrdata[SSL3_RANDOM_LENGTH * 2];
 
             if ((mechanism == CKM_TLS12_MASTER_KEY_DERIVE) ||
                 (mechanism == CKM_TLS12_MASTER_KEY_DERIVE_DH)) {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_TLS12_MASTER_KEY_DERIVE_PARAMS))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 CK_TLS12_MASTER_KEY_DERIVE_PARAMS *tls12_master =
                     (CK_TLS12_MASTER_KEY_DERIVE_PARAMS *)pMechanism->pParameter;
                 tlsPrfHash = GetHashTypeFromMechanism(tls12_master->prfHashMechanism);
                 if (tlsPrfHash == HASH_AlgNULL) {
                     crv = CKR_MECHANISM_PARAM_INVALID;
                     break;
                 }
             } else if ((mechanism == CKM_NSS_TLS_MASTER_KEY_DERIVE_SHA256) ||
@@ -7063,16 +7119,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             CK_SSL3_KEY_MAT_PARAMS *ssl3_keys;
             CK_SSL3_KEY_MAT_OUT *ssl3_keys_out;
             CK_ULONG effKeySize;
             unsigned int block_needed;
             unsigned char srcrdata[SSL3_RANDOM_LENGTH * 2];
             unsigned char crsrdata[SSL3_RANDOM_LENGTH * 2];
 
             if (mechanism == CKM_TLS12_KEY_AND_MAC_DERIVE) {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_TLS12_KEY_MAT_PARAMS))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 CK_TLS12_KEY_MAT_PARAMS *tls12_keys =
                     (CK_TLS12_KEY_MAT_PARAMS *)pMechanism->pParameter;
                 tlsPrfHash = GetHashTypeFromMechanism(tls12_keys->prfHashMechanism);
                 if (tlsPrfHash == HASH_AlgNULL) {
                     crv = CKR_MECHANISM_PARAM_INVALID;
                     break;
                 }
             } else if (mechanism == CKM_NSS_TLS_KEY_AND_MAC_DERIVE_SHA256) {
@@ -7106,16 +7166,23 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
                 break;
             }
             sha = SHA1_NewContext();
             if (sha == NULL) {
                 MD5_DestroyContext(md5, PR_TRUE);
                 crv = CKR_HOST_MEMORY;
                 break;
             }
+
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_SSL3_KEY_MAT_PARAMS))) {
+                MD5_DestroyContext(md5, PR_TRUE);
+                SHA1_DestroyContext(sha, PR_TRUE);
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             ssl3_keys = (CK_SSL3_KEY_MAT_PARAMS *)pMechanism->pParameter;
 
             PORT_Memcpy(srcrdata,
                         ssl3_keys->RandomInfo.pServerRandom, SSL3_RANDOM_LENGTH);
             PORT_Memcpy(srcrdata + SSL3_RANDOM_LENGTH,
                         ssl3_keys->RandomInfo.pClientRandom, SSL3_RANDOM_LENGTH);
 
             PORT_Memcpy(crsrdata,
@@ -7300,16 +7367,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             unsigned char des3key[MAX_DES3_KEY_SIZE];
             CK_DES_CBC_ENCRYPT_DATA_PARAMS *desEncryptPtr;
             int mode;
             unsigned char *iv;
             unsigned char *data;
             CK_ULONG len;
 
             if (mechanism == CKM_DES3_ECB_ENCRYPT_DATA) {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)
                                 pMechanism->pParameter;
                 mode = NSS_DES_EDE3;
                 iv = NULL;
                 data = stringPtr->pData;
                 len = stringPtr->ulLen;
             } else {
                 mode = NSS_DES_EDE3_CBC;
@@ -7349,20 +7420,28 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             int mode;
             unsigned char *iv;
             unsigned char *data;
             CK_ULONG len;
 
             if (mechanism == CKM_AES_ECB_ENCRYPT_DATA) {
                 mode = NSS_AES;
                 iv = NULL;
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)pMechanism->pParameter;
                 data = stringPtr->pData;
                 len = stringPtr->ulLen;
             } else {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_AES_CBC_ENCRYPT_DATA_PARAMS))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 aesEncryptPtr =
                     (CK_AES_CBC_ENCRYPT_DATA_PARAMS *)pMechanism->pParameter;
                 mode = NSS_AES_CBC;
                 iv = aesEncryptPtr->iv;
                 data = aesEncryptPtr->pData;
                 len = aesEncryptPtr->length;
             }
 
@@ -7385,24 +7464,32 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             void *cipherInfo;
             CK_AES_CBC_ENCRYPT_DATA_PARAMS *aesEncryptPtr;
             int mode;
             unsigned char *iv;
             unsigned char *data;
             CK_ULONG len;
 
             if (mechanism == CKM_CAMELLIA_ECB_ENCRYPT_DATA) {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)
                                 pMechanism->pParameter;
                 aesEncryptPtr = NULL;
                 mode = NSS_CAMELLIA;
                 data = stringPtr->pData;
                 len = stringPtr->ulLen;
                 iv = NULL;
             } else {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_AES_CBC_ENCRYPT_DATA_PARAMS))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 stringPtr = NULL;
                 aesEncryptPtr = (CK_AES_CBC_ENCRYPT_DATA_PARAMS *)
                                     pMechanism->pParameter;
                 mode = NSS_CAMELLIA_CBC;
                 iv = aesEncryptPtr->iv;
                 data = aesEncryptPtr->pData;
                 len = aesEncryptPtr->length;
             }
@@ -7426,24 +7513,32 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             void *cipherInfo;
             CK_AES_CBC_ENCRYPT_DATA_PARAMS *aesEncryptPtr;
             int mode;
             unsigned char *iv;
             unsigned char *data;
             CK_ULONG len;
 
             if (mechanism == CKM_SEED_ECB_ENCRYPT_DATA) {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 mode = NSS_SEED;
                 stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)
                                 pMechanism->pParameter;
                 aesEncryptPtr = NULL;
                 data = stringPtr->pData;
                 len = stringPtr->ulLen;
                 iv = NULL;
             } else {
+                if (BAD_PARAM_CAST(pMechanism, sizeof(CK_AES_CBC_ENCRYPT_DATA_PARAMS))) {
+                    crv = CKR_MECHANISM_PARAM_INVALID;
+                    break;
+                }
                 mode = NSS_SEED_CBC;
                 aesEncryptPtr = (CK_AES_CBC_ENCRYPT_DATA_PARAMS *)
                                     pMechanism->pParameter;
                 iv = aesEncryptPtr->iv;
                 data = aesEncryptPtr->pData;
                 len = aesEncryptPtr->length;
             }
 
@@ -7525,16 +7620,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             break;
         }
 
         case CKM_CONCATENATE_BASE_AND_DATA:
             crv = sftk_DeriveSensitiveCheck(sourceKey, key);
             if (crv != CKR_OK)
                 break;
 
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)pMechanism->pParameter;
             tmpKeySize = att->attrib.ulValueLen + stringPtr->ulLen;
             if (keySize == 0)
                 keySize = tmpKeySize;
             if (keySize > tmpKeySize) {
                 crv = CKR_TEMPLATE_INCONSISTENT;
                 break;
             }
@@ -7551,16 +7650,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             crv = sftk_forceAttribute(key, CKA_VALUE, buf, keySize);
             PORT_ZFree(buf, tmpKeySize);
             break;
         case CKM_CONCATENATE_DATA_AND_BASE:
             crv = sftk_DeriveSensitiveCheck(sourceKey, key);
             if (crv != CKR_OK)
                 break;
 
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)pMechanism->pParameter;
             tmpKeySize = att->attrib.ulValueLen + stringPtr->ulLen;
             if (keySize == 0)
                 keySize = tmpKeySize;
             if (keySize > tmpKeySize) {
                 crv = CKR_TEMPLATE_INCONSISTENT;
                 break;
             }
@@ -7577,16 +7680,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
             crv = sftk_forceAttribute(key, CKA_VALUE, buf, keySize);
             PORT_ZFree(buf, tmpKeySize);
             break;
         case CKM_XOR_BASE_AND_DATA:
             crv = sftk_DeriveSensitiveCheck(sourceKey, key);
             if (crv != CKR_OK)
                 break;
 
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_KEY_DERIVATION_STRING_DATA))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             stringPtr = (CK_KEY_DERIVATION_STRING_DATA *)pMechanism->pParameter;
             tmpKeySize = PR_MIN(att->attrib.ulValueLen, stringPtr->ulLen);
             if (keySize == 0)
                 keySize = tmpKeySize;
             if (keySize > tmpKeySize) {
                 crv = CKR_TEMPLATE_INCONSISTENT;
                 break;
             }
@@ -7601,16 +7708,20 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
                 buf[i] ^= stringPtr->pData[i];
             }
 
             crv = sftk_forceAttribute(key, CKA_VALUE, buf, keySize);
             PORT_ZFree(buf, keySize);
             break;
 
         case CKM_EXTRACT_KEY_FROM_KEY: {
+            if (BAD_PARAM_CAST(pMechanism, sizeof(CK_EXTRACT_PARAMS))) {
+                crv = CKR_MECHANISM_PARAM_INVALID;
+                break;
+            }
             /* the following assumes 8 bits per byte */
             CK_ULONG extract = *(CK_EXTRACT_PARAMS *)pMechanism->pParameter;
             CK_ULONG shift = extract & 0x7; /* extract mod 8 the fast way */
             CK_ULONG offset = extract >> 3; /* extract div 8 the fast way */
 
             crv = sftk_DeriveSensitiveCheck(sourceKey, key);
             if (crv != CKR_OK)
                 break;