Title: Memory Leak? CKey::SetCompactSignature Post by: bytemaster on May 19, 2013, 04:31:38 PM In CKey::SetCompactSignature line 370 of key.cpp I believe a call to ECDSA_SIG_free(sig) is needed.
If ECDSA_SIG_recover_key_GFp fails, then ECDSA_SIG_free() is never called on the ECDSA_SIG_new() allocated on line 353. I suspect that this doesn't fail very often, but could be an attack vector. Not being an expert in the OpenSSL API I am not sure if this memory is freed someplace else. Title: Re: Memory Leak? CKey::SetCompactSignature Post by: bytemaster on May 19, 2013, 04:37:51 PM bool CKey::SetCompactSignature(uint256 hash, const std::vector<unsigned char>& vchSig)
{ if (vchSig.size() != 65) return false; int nV = vchSig[0]; if (nV<27 || nV>=35) return false; ECDSA_SIG *sig = ECDSA_SIG_new(); BN_bin2bn(&vchSig[1],32,sig->r); BN_bin2bn(&vchSig[33],32,sig->s); EC_KEY_free(pkey); pkey = EC_KEY_new_by_curve_name(NID_secp256k1); if (nV >= 31) { SetCompressedPubKey(); nV -= 4; } if (ECDSA_SIG_recover_key_GFp(pkey, sig, (unsigned char*)&hash, sizeof(hash), nV - 27, 0) == 1) { fSet = true; ECDSA_SIG_free(sig); return true; } ECDSA_SIG_free(sig); // Should this be added to fix leak? return false; } Title: Re: Memory Leak? CKey::SetCompactSignature Post by: wumpus on May 19, 2013, 04:40:17 PM Indeed, looks that way, good catch.
You should really create this as a github issue so that it doesn't get lost in the avalanche here. Title: Re: Memory Leak? CKey::SetCompactSignature Post by: bytemaster on May 19, 2013, 04:49:02 PM I created a pull request with this patch in it.
Title: Re: Memory Leak? CKey::SetCompactSignature Post by: wumpus on May 19, 2013, 05:25:27 PM Thanks :)
|