Bug 1275197 - Ensure nsNSSU2FToken.cpp GetSymKeyByNickname() does not cause leaks. r=keeler
Prior to these changes, GetSymKeyByNickname() could theoretically leak. This
should not happen in practice, so the changes here just ensure that the code
doesn't cause leaks.
MozReview-Commit-ID: LWtqLmsBPV2
--- a/security/manager/ssl/nsNSSU2FToken.cpp
+++ b/security/manager/ssl/nsNSSU2FToken.cpp
@@ -79,48 +79,59 @@ nsNSSU2FToken::virtualDestroyNSSReferenc
}
void
nsNSSU2FToken::destructorSafeDestroyNSSReference()
{
mWrappingKey = nullptr;
}
+/**
+ * Gets the first key with the given nickname from the given slot. Any other
+ * keys found are not returned.
+ * PK11_GetNextSymKey() should not be called on the returned key.
+ *
+ * @param aSlot Slot to search.
+ * @param aNickname Nickname the key should have.
+ * @return The first key found. nullptr if no key could be found.
+ */
static UniquePK11SymKey
GetSymKeyByNickname(const UniquePK11SlotInfo& aSlot,
- nsCString aNickname,
+ const nsCString& aNickname,
const nsNSSShutDownPreventionLock&)
{
MOZ_ASSERT(aSlot);
if (!aSlot) {
return nullptr;
}
MOZ_LOG(gNSSTokenLog, LogLevel::Debug,
("Searching for a symmetric key named %s", aNickname.get()));
- PK11SymKey* keyList;
- keyList = PK11_ListFixedKeysInSlot(aSlot.get(),
- const_cast<char*>(aNickname.get()),
- /* wincx */ nullptr);
- while (keyList) {
- UniquePK11SymKey freeKey(keyList);
-
- UniquePORTString freeKeyName(PK11_GetSymKeyNickname(freeKey.get()));
-
- if (aNickname == freeKeyName.get()) {
- MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key found!"));
- return freeKey;
- }
-
- keyList = PK11_GetNextSymKey(keyList);
+ UniquePK11SymKey keyListHead(
+ PK11_ListFixedKeysInSlot(aSlot.get(), const_cast<char*>(aNickname.get()),
+ /* wincx */ nullptr));
+ if (!keyListHead) {
+ MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key not found."));
+ return nullptr;
}
- MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key not found."));
- return nullptr;
+ // Sanity check PK11_ListFixedKeysInSlot() only returns keys with the correct
+ // nickname.
+ MOZ_ASSERT(aNickname ==
+ UniquePORTString(PK11_GetSymKeyNickname(keyListHead.get())).get());
+ MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key found!"));
+
+ // Free any remaining keys in the key list.
+ UniquePK11SymKey freeKey(PK11_GetNextSymKey(keyListHead.get()));
+ while (freeKey) {
+ freeKey = UniquePK11SymKey(PK11_GetNextSymKey(freeKey.get()));
+ }
+
+ return keyListHead;
}
static nsresult
GenEcKeypair(const UniquePK11SlotInfo& aSlot,
/*out*/ UniqueSECKEYPrivateKey& aPrivKey,
/*out*/ UniqueSECKEYPublicKey& aPubKey,
const nsNSSShutDownPreventionLock&)
{