From 6f50dbd2fdeef7bc24317a487936502e25a05de9 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 9 Feb 2015 15:28:35 -0500 Subject: [PATCH 1/2] Fix NegateSignatureS to not duplicate last byte of S NegateSignatureS is called with a signature without a hashtype, so do not save the last byte and append it after S negation. Updates the two tests which were affected by this bug. --- src/test/data/script_invalid.json | 2 +- src/test/data/script_valid.json | 2 +- src/test/script_tests.cpp | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/data/script_invalid.json b/src/test/data/script_invalid.json index a67c157af..3b2f64d76 100644 --- a/src/test/data/script_invalid.json +++ b/src/test/data/script_invalid.json @@ -696,7 +696,7 @@ "BIP66 example 11, with DERSIG" ], [ - "0x49 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef05101", + "0x48 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef001", "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG", "LOW_S", "P2PK with high S" diff --git a/src/test/data/script_valid.json b/src/test/data/script_valid.json index fb81fcb1f..2b4b0989b 100644 --- a/src/test/data/script_valid.json +++ b/src/test/data/script_valid.json @@ -814,7 +814,7 @@ "BIP66 example 12, with DERSIG" ], [ - "0x49 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef05101", + "0x48 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef001", "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG", "", "P2PK with high S but no LOW_S" diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 6092afd78..35a7f6639 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -107,7 +107,6 @@ void static NegateSignatureS(std::vector& vchSig) { std::vector r, s; r = std::vector(vchSig.begin() + 4, vchSig.begin() + 4 + vchSig[3]); s = std::vector(vchSig.begin() + 6 + vchSig[3], vchSig.begin() + 6 + vchSig[3] + vchSig[5 + vchSig[3]]); - unsigned char hashtype = vchSig.back(); // Really ugly to implement mod-n negation here, but it would be feature creep to expose such functionality from libsecp256k1. static const unsigned char order[33] = { @@ -141,7 +140,6 @@ void static NegateSignatureS(std::vector& vchSig) { vchSig.push_back(0x02); vchSig.push_back(s.size()); vchSig.insert(vchSig.end(), s.begin(), s.end()); - vchSig.push_back(hashtype); } namespace From 78c6bedb9cd3ed85f829a288146533e3b41f784b Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 10 Feb 2015 12:11:59 -0500 Subject: [PATCH 2/2] Add test for DER-encoding edge case The fix to NegateSignatureS caused a test which had been failing in IsValidSignatureEncoding to then fail in IsLowDERSignature. Add new test so the original check remains exercised. --- src/test/data/script_invalid.json | 6 ++++++ src/test/data/script_valid.json | 6 ++++++ src/test/script_tests.cpp | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/src/test/data/script_invalid.json b/src/test/data/script_invalid.json index 3b2f64d76..3c52547a6 100644 --- a/src/test/data/script_invalid.json +++ b/src/test/data/script_invalid.json @@ -695,6 +695,12 @@ "DERSIG", "BIP66 example 11, with DERSIG" ], +[ + "0x48 0x304402203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022054e1c258c2981cdfba5df1f46661fb6541c44f77ca0092f3600331abfffb12510101", + "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG", + "DERSIG", + "P2PK with multi-byte hashtype, with DERSIG" +], [ "0x48 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef001", "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG", diff --git a/src/test/data/script_valid.json b/src/test/data/script_valid.json index 2b4b0989b..34e2c8d61 100644 --- a/src/test/data/script_valid.json +++ b/src/test/data/script_valid.json @@ -813,6 +813,12 @@ "DERSIG", "BIP66 example 12, with DERSIG" ], +[ + "0x48 0x304402203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022054e1c258c2981cdfba5df1f46661fb6541c44f77ca0092f3600331abfffb12510101", + "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG", + "", + "P2PK with multi-byte hashtype, without DERSIG" +], [ "0x48 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef001", "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG", diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 35a7f6639..e410b5971 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -476,6 +476,12 @@ BOOST_AUTO_TEST_CASE(script_build) good.push_back(TestBuilder(CScript() << OP_2 << ToByteVector(keys.pubkey1C) << ToByteVector(keys.pubkey2C) << OP_2 << OP_CHECKMULTISIG << OP_NOT, "BIP66 example 12, with DERSIG", SCRIPT_VERIFY_DERSIG ).Num(0).PushSig(keys.key1, SIGHASH_ALL, 33, 32).EditPush(1, "45022100", "440220").Num(0)); + good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey2C) << OP_CHECKSIG, + "P2PK with multi-byte hashtype, without DERSIG", 0 + ).PushSig(keys.key2, SIGHASH_ALL).EditPush(70, "01", "0101")); + bad.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey2C) << OP_CHECKSIG, + "P2PK with multi-byte hashtype, with DERSIG", SCRIPT_VERIFY_DERSIG + ).PushSig(keys.key2, SIGHASH_ALL).EditPush(70, "01", "0101")); good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey2C) << OP_CHECKSIG, "P2PK with high S but no LOW_S", 0