From c6b7b29f232c651f898eeffb93f36c8f537c56d2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sun, 11 Jan 2015 11:08:53 +0100 Subject: [PATCH] Improve robustness of DER recoding code Add some defensive programming on top of #5634. This copies the respective OpenSSL code in ECDSA_verify in OpenSSL pre-1.0.1k (e.g. https://github.com/openssl/openssl/blob/OpenSSL_1_0_1j/crypto/ecdsa/ecs_vrf.c#L89) more closely. As reported by @sergiodemianlerner. --- src/ecwrapper.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/ecwrapper.cpp b/src/ecwrapper.cpp index 0236e90c1..5e3aec25b 100644 --- a/src/ecwrapper.cpp +++ b/src/ecwrapper.cpp @@ -124,7 +124,18 @@ bool CECKey::Verify(const uint256 &hash, const std::vector& vchSi unsigned char *norm_der = NULL; ECDSA_SIG *norm_sig = ECDSA_SIG_new(); const unsigned char* sigptr = &vchSig[0]; - d2i_ECDSA_SIG(&norm_sig, &sigptr, vchSig.size()); + assert(norm_sig); + if (d2i_ECDSA_SIG(&norm_sig, &sigptr, vchSig.size()) == NULL) + { + /* As of OpenSSL 1.0.0p d2i_ECDSA_SIG frees and nulls the pointer on + * error. But OpenSSL's own use of this function redundantly frees the + * result. As ECDSA_SIG_free(NULL) is a no-op, and in the absence of a + * clear contract for the function behaving the same way is more + * conservative. + */ + ECDSA_SIG_free(norm_sig); + return false; + } int derlen = i2d_ECDSA_SIG(norm_sig, &norm_der); ECDSA_SIG_free(norm_sig); if (derlen <= 0)