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

security/manager/ssl/nsNSSU2FToken.cpp

@@ -84,9 +84,18 @@ 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);
@@ -97,25 +106,27 @@ GetSymKeyByNickname(const UniquePK11SlotInfo& aSlot,
   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()));
+  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;
+  }
 
-    if (aNickname == freeKeyName.get()) {
-      MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key found!"));
-      return freeKey;
-    }
+  // 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!"));
 
-    keyList = PK11_GetNextSymKey(keyList);
+  // Free any remaining keys in the key list.
+  UniquePK11SymKey freeKey(PK11_GetNextSymKey(keyListHead.get()));
+  while (freeKey) {
+    freeKey = UniquePK11SymKey(PK11_GetNextSymKey(freeKey.get()));
   }
 
-  MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key not found."));
-  return nullptr;
+  return keyListHead;
 }
 
 static nsresult