Memory Leak? CKey::SetCompactSignature

[bytemaster]

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.

[bytemaster]

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;
}

[wumpus]

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.

[bytemaster]

I created a pull request with this patch in it.

[wumpus]

Thanks