bug 1234417 - fix a leak in CreateECPublicKey r=rbarnes
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 21 Dec 2015 17:14:41 -0800
changeset 312859 f745c6e024a325921801db0394add38c86f34add
parent 312858 3242bee2d9568ac74d4c095dc7d2fac4798a02ca
child 312860 77e9d0590464edd3f30d7716396cb67a57c7aaf8
push id5703
push userraliiev@mozilla.com
push dateMon, 07 Mar 2016 14:18:41 +0000
treeherdermozilla-beta@31e373ad5b5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrbarnes
bugs1234417
milestone46.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1234417 - fix a leak in CreateECPublicKey r=rbarnes Before this patch, CreateECPublicKey would create a SECKEYPublicKey allocated on a scoped arena. It would then call CryptoKey::PublicKeyValid, which has the side-effect of importing the key to the internal PKCS#11 slot. When the arena went out of scope, the memory for the key would be released, but the reference to the slot wouldn't, causing a leak. This patch fixes the leak by making the SECKEYPublicKey a ScopedSECKEYPublicKey (which ensures that the type-specific "destructor" SECKEY_DestroyPublicKey is called, which releases the reference to the PKCS#11 slot).
dom/crypto/CryptoKey.cpp
--- a/dom/crypto/CryptoKey.cpp
+++ b/dom/crypto/CryptoKey.cpp
@@ -969,21 +969,26 @@ CryptoKey::PrivateKeyToJwk(SECKEYPrivate
 SECKEYPublicKey*
 CreateECPublicKey(const SECItem* aKeyData, const nsString& aNamedCurve)
 {
   ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return nullptr;
   }
 
-  SECKEYPublicKey* key = PORT_ArenaZNew(arena, SECKEYPublicKey);
+  // It's important that this be a ScopedSECKEYPublicKey, as this ensures that
+  // SECKEY_DestroyPublicKey will be called on it. If this doesn't happen, when
+  // CryptoKey::PublicKeyValid is called on it and it gets moved to the internal
+  // PKCS#11 slot, it will leak a reference to the slot.
+  ScopedSECKEYPublicKey key(PORT_ArenaZNew(arena, SECKEYPublicKey));
   if (!key) {
     return nullptr;
   }
 
+  key->arena = nullptr; // key doesn't own the arena; it won't get double-freed
   key->keyType = ecKey;
   key->pkcs11Slot = nullptr;
   key->pkcs11ID = CK_INVALID_HANDLE;
 
   // Create curve parameters.
   SECItem* params = CreateECParamsForCurve(aNamedCurve, arena);
   if (!params) {
     return nullptr;