Remove unneeded lax ECDSA signature verification.

Signed-off-by: Daira Hopwood <[email protected]>

diff --git a/src/pubkey.cpp b/src/pubkey.cpp
index 949adbb9300dbdcdabe1ca848307207f4675ddc5..6e50a133407a685d8a4c3fb36e6a6e2d26bdfe07 100644 (file)
--- a/src/pubkey.cpp
+++ b/src/pubkey.cpp
@@ -14,158 +14,6 @@ namespace
 secp256k1_context* secp256k1_context_verify = NULL;
 }
 
-/** This function is taken from the libsecp256k1 distribution and implements
- *  DER parsing for ECDSA signatures, while supporting an arbitrary subset of
- *  format violations.
- *
- *  Supported violations include negative integers, excessive padding, garbage
- *  at the end, and overly long length descriptors. This is safe to use in
- *  Bitcoin because since the activation of BIP66, signatures are verified to be
- *  strict DER before being passed to this module, and we know it supports all
- *  violations present in the blockchain before that point.
- */
-static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) {
-    size_t rpos, rlen, spos, slen;
-    size_t pos = 0;
-    size_t lenbyte;
-    unsigned char tmpsig[64] = {0};
-    int overflow = 0;
-
-    /* Hack to initialize sig with a correctly-parsed but invalid signature. */
-    secp256k1_ecdsa_signature_parse_compact(ctx, sig, tmpsig);
-
-    /* Sequence tag byte */
-    if (pos == inputlen || input[pos] != 0x30) {
-        return 0;
-    }
-    pos++;
-
-    /* Sequence length bytes */
-    if (pos == inputlen) {
-        return 0;
-    }
-    lenbyte = input[pos++];
-    if (lenbyte & 0x80) {
-        lenbyte -= 0x80;
-        if (lenbyte > inputlen - pos) {
-            return 0;
-        }
-        pos += lenbyte;
-    }
-
-    /* Integer tag byte for R */
-    if (pos == inputlen || input[pos] != 0x02) {
-        return 0;
-    }
-    pos++;
-
-    /* Integer length for R */
-    if (pos == inputlen) {
-        return 0;
-    }
-    lenbyte = input[pos++];
-    if (lenbyte & 0x80) {
-        lenbyte -= 0x80;
-        if (lenbyte > inputlen - pos) {
-            return 0;
-        }
-        while (lenbyte > 0 && input[pos] == 0) {
-            pos++;
-            lenbyte--;
-        }
-        static_assert(sizeof(size_t) >= 4, "size_t too small");
-        if (lenbyte >= 4) {
-            return 0;
-        }
-        rlen = 0;
-        while (lenbyte > 0) {
-            rlen = (rlen << 8) + input[pos];
-            pos++;
-            lenbyte--;
-        }
-    } else {
-        rlen = lenbyte;
-    }
-    if (rlen > inputlen - pos) {
-        return 0;
-    }
-    rpos = pos;
-    pos += rlen;
-
-    /* Integer tag byte for S */
-    if (pos == inputlen || input[pos] != 0x02) {
-        return 0;
-    }
-    pos++;
-
-    /* Integer length for S */
-    if (pos == inputlen) {
-        return 0;
-    }
-    lenbyte = input[pos++];
-    if (lenbyte & 0x80) {
-        lenbyte -= 0x80;
-        if (lenbyte > inputlen - pos) {
-            return 0;
-        }
-        while (lenbyte > 0 && input[pos] == 0) {
-            pos++;
-            lenbyte--;
-        }
-        static_assert(sizeof(size_t) >= 4, "size_t too small");
-        if (lenbyte >= 4) {
-            return 0;
-        }
-        slen = 0;
-        while (lenbyte > 0) {
-            slen = (slen << 8) + input[pos];
-            pos++;
-            lenbyte--;
-        }
-    } else {
-        slen = lenbyte;
-    }
-    if (slen > inputlen - pos) {
-        return 0;
-    }
-    spos = pos;
-    pos += slen;
-
-    /* Ignore leading zeroes in R */
-    while (rlen > 0 && input[rpos] == 0) {
-        rlen--;
-        rpos++;
-    }
-    /* Copy R value */
-    if (rlen > 32) {
-        overflow = 1;
-    } else {
-        memcpy(tmpsig + 32 - rlen, input + rpos, rlen);
-    }
-
-    /* Ignore leading zeroes in S */
-    while (slen > 0 && input[spos] == 0) {
-        slen--;
-        spos++;
-    }
-    /* Copy S value */
-    if (slen > 32) {
-        overflow = 1;
-    } else {
-        memcpy(tmpsig + 64 - slen, input + spos, slen);
-    }
-
-    if (!overflow) {
-        overflow = !secp256k1_ecdsa_signature_parse_compact(ctx, sig, tmpsig);
-    }
-    if (overflow) {
-        /* Overwrite the result again with a correctly-parsed but invalid
-           signature if parsing failed. */
-        memset(tmpsig, 0, 64);
-        secp256k1_ecdsa_signature_parse_compact(ctx, sig, tmpsig);
-    }
-    return 1;
-}
 
 bool CPubKey::Verify(const uint256 &hash, const std::vector<unsigned char>& vchSig) const {
     if (!IsValid())
@@ -178,11 +26,12 @@ bool CPubKey::Verify(const uint256 &hash, const std::vector<unsigned char>& vchS
     if (vchSig.size() == 0) {
         return false;
     }
-    if (!ecdsa_signature_parse_der_lax(secp256k1_context_verify, &sig, &vchSig[0], vchSig.size())) {
+    /* Zcash, unlike Bitcoin, has always enforced strict DER signatures. */
+    if (!secp256k1_ecdsa_signature_parse_der(secp256k1_context_verify, &sig, &vchSig[0], vchSig.size())) {
         return false;
     }
     /* libsecp256k1's ECDSA verification requires lower-S signatures, which have
-     * not historically been enforced in Bitcoin, so normalize them first. */
+     * not historically been enforced in Bitcoin or Zcash, so normalize them first. */
     secp256k1_ecdsa_signature_normalize(secp256k1_context_verify, &sig, &sig);
     return secp256k1_ecdsa_verify(secp256k1_context_verify, &sig, hash.begin(), &pubkey);
 }
@@ -277,7 +126,9 @@ bool CExtPubKey::Derive(CExtPubKey &out, unsigned int nChild) const {
 
 /* static */ bool CPubKey::CheckLowS(const std::vector<unsigned char>& vchSig) {
     secp256k1_ecdsa_signature sig;
-    if (!ecdsa_signature_parse_der_lax(secp256k1_context_verify, &sig, &vchSig[0], vchSig.size())) {
+
+    /* Zcash, unlike Bitcoin, has always enforced strict DER signatures. */
+    if (!secp256k1_ecdsa_signature_parse_der(secp256k1_context_verify, &sig, &vchSig[0], vchSig.size())) {
         return false;
     }
     return (!secp256k1_ecdsa_signature_normalize(secp256k1_context_verify, NULL, &sig));