Improve robustness of DER recoding code

laanwj · laanwj · commit c6b7b29f232c · 2015-01-12T08:59:03.000+01:00
Add some defensive programming on top of dashpay#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.
diff --git 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)
