diff options
| author | Wladimir J. van der Laan <laanwj@gmail.com> | 2015-01-11 11:08:53 +0100 |
|---|---|---|
| committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2015-01-12 09:12:12 +0100 |
| commit | 12b7c444f0b0bee42e97b62b0b40f11c8d1debe7 (patch) | |
| tree | 9c55c75c60ce0e68a363ac6aa7a50dc12a9b1991 | |
| parent | 76ce5c8de3b97c9843e23c9b30f1049045c4b293 (diff) | |
| download | bitcoin-12b7c444f0b0bee42e97b62b0b40f11c8d1debe7.tar.xz | |
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.
Github-Pull: #5640
Rebased-From: c6b7b29f232c651f898eeffb93f36c8f537c56d2
| -rw-r--r-- | src/ecwrapper.cpp | 13 |
1 files changed, 12 insertions, 1 deletions
diff --git a/src/ecwrapper.cpp b/src/ecwrapper.cpp index 79f70d1f47..5bfa740cc6 100644 --- a/src/ecwrapper.cpp +++ b/src/ecwrapper.cpp @@ -124,7 +124,18 @@ bool CECKey::Verify(const uint256 &hash, const std::vector<unsigned char>& 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) |