From 377d226ef196247115eff2c24df56d0b3d9451f1 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Tue, 27 Oct 2015 23:10:54 +0000 Subject: [PATCH] Enforce the LOW_S script validation flag --- .../bitcoinj/crypto/TransactionSignature.java | 28 +++++++++++++++++-- .../main/java/org/bitcoinj/script/Script.java | 4 +-- .../org/bitcoinj/script/tx_invalid.json | 4 +++ .../org/bitcoinj/script/tx_valid.json | 8 ++++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/crypto/TransactionSignature.java b/core/src/main/java/org/bitcoinj/crypto/TransactionSignature.java index 14a88920..4068475a 100644 --- a/core/src/main/java/org/bitcoinj/crypto/TransactionSignature.java +++ b/core/src/main/java/org/bitcoinj/crypto/TransactionSignature.java @@ -156,11 +156,32 @@ public class TransactionSignature extends ECKey.ECDSASignature { /** * Returns a decoded signature. + * + * @param requireCanonicalEncoding if the encoding of the signature must + * be canonical. + * @throws RuntimeException if the signature is invalid or unparseable in some way. + * @deprecated use {@link #decodeFromBitcoin(byte[], boolean, boolean} instead}. + */ + @Deprecated + public static TransactionSignature decodeFromBitcoin(byte[] bytes, + boolean requireCanonicalEncoding) throws VerificationException { + return decodeFromBitcoin(bytes, requireCanonicalEncoding, false); + } + + /** + * Returns a decoded signature. + * + * @param requireCanonicalEncoding if the encoding of the signature must + * be canonical. + * @param requireCanonicalSValue if the S-value must be canonical (below half + * the order of the curve). * @throws RuntimeException if the signature is invalid or unparseable in some way. */ - public static TransactionSignature decodeFromBitcoin(byte[] bytes, boolean requireCanonical) throws VerificationException { + public static TransactionSignature decodeFromBitcoin(byte[] bytes, + boolean requireCanonicalEncoding, + boolean requireCanonicalSValue) throws VerificationException { // Bitcoin encoding is DER signature + sighash byte. - if (requireCanonical && !isEncodingCanonical(bytes)) + if (requireCanonicalEncoding && !isEncodingCanonical(bytes)) throw new VerificationException("Signature encoding is not canonical."); ECKey.ECDSASignature sig; try { @@ -168,6 +189,9 @@ public class TransactionSignature extends ECKey.ECDSASignature { } catch (IllegalArgumentException e) { throw new VerificationException("Could not decode DER", e); } + if (requireCanonicalSValue && !sig.isCanonical()) + throw new VerificationException("S-value is not canonical."); + // In Bitcoin, any value of the final byte is valid, but not necessarily canonical. See javadocs for // isEncodingCanonical to learn more about this. So we must store the exact byte found. return new TransactionSignature(sig.r, sig.s, bytes[bytes.length - 1]); diff --git a/core/src/main/java/org/bitcoinj/script/Script.java b/core/src/main/java/org/bitcoinj/script/Script.java index 0837c2ce..d99a8380 100644 --- a/core/src/main/java/org/bitcoinj/script/Script.java +++ b/core/src/main/java/org/bitcoinj/script/Script.java @@ -1320,8 +1320,8 @@ public class Script { // TODO: Use int for indexes everywhere, we can't have that many inputs/outputs boolean sigValid = false; try { - // TODO: Should pass through LOW_S verification flag - TransactionSignature sig = TransactionSignature.decodeFromBitcoin(sigBytes, requireCanonical); + TransactionSignature sig = TransactionSignature.decodeFromBitcoin(sigBytes, requireCanonical, + verifyFlags.contains(VerifyFlag.LOW_S)); // TODO: Should check hash type is known Sha256Hash hash = txContainingThis.hashForSignature(index, connectedScript, (byte) sig.sighashFlags); diff --git a/core/src/test/resources/org/bitcoinj/script/tx_invalid.json b/core/src/test/resources/org/bitcoinj/script/tx_invalid.json index 3bc0f611..5242a045 100644 --- a/core/src/test/resources/org/bitcoinj/script/tx_invalid.json +++ b/core/src/test/resources/org/bitcoinj/script/tx_invalid.json @@ -113,5 +113,9 @@ [[["b1dbc81696c8a9c0fccd0693ab66d7c368dbc38c0def4e800685560ddd1b2132", 0, "DUP HASH160 0x14 0x4b3bd7eba3bc0284fd3007be7f3be275e94f5826 EQUALVERIFY CHECKSIG"]], "010000000132211bdd0d568506804eef0d8cc3db68c3d766ab9306cdfcc0a9c89616c8dbb1000000006c493045022100c7bb0faea0522e74ff220c20c022d2cb6033f8d167fb89e75a50e237a35fd6d202203064713491b1f8ad5f79e623d0219ad32510bfaa1009ab30cbee77b59317d6e30001210237af13eb2d84e4545af287b919c2282019c9691cc509e78e196a9d8274ed1be0ffffffff0100000000000000001976a914f1b3ed2eda9a2ebe5a9374f692877cdf87c0f95b88ac00000000", "P2SH,DERSIG"], +["A transaction with a high-S signature."], +[[["a54b3bff74c910adff4f81b641587440c9f3b02a67c69b06d4eea36c0d199bac", 0, "DUP HASH160 0x14 0x33b9fe6942f268ae728136046e638c8863bed91f EQUALVERIFY CHECKSIG"]], +"0100000001ac9b190d6ca3eed4069bc6672ab0f3c940745841b6814fffad10c974ff3b4ba5000000006c493046022100c16257354c7ec7fedda7ce724abfc6a8f26fd433d08b1a11bd01a8f0f157b1fb0221009069419fa4454cd52f1d3219c395b96c3dc0404257196f96d819d71e4f95b8290121039770779101a01dbbf6c25466df8869e648dc75a51d64ebf5136c824c32fcc9c8ffffffff0100000000000000001976a914aaf0529f46a86271255621ed566e79e67345916d88ac00000000", "P2SH,LOW_S"], + ["Make diffs cleaner by leaving a comment here without comma at the end"] ] diff --git a/core/src/test/resources/org/bitcoinj/script/tx_valid.json b/core/src/test/resources/org/bitcoinj/script/tx_valid.json index 95741458..89580500 100644 --- a/core/src/test/resources/org/bitcoinj/script/tx_valid.json +++ b/core/src/test/resources/org/bitcoinj/script/tx_valid.json @@ -181,5 +181,13 @@ [[["b1dbc81696c8a9c0fccd0693ab66d7c368dbc38c0def4e800685560ddd1b2132", 0, "DUP HASH160 0x14 0x4b3bd7eba3bc0284fd3007be7f3be275e94f5826 EQUALVERIFY CHECKSIG"]], "010000000132211bdd0d568506804eef0d8cc3db68c3d766ab9306cdfcc0a9c89616c8dbb1000000006c493045022100c7bb0faea0522e74ff220c20c022d2cb6033f8d167fb89e75a50e237a35fd6d202203064713491b1f8ad5f79e623d0219ad32510bfaa1009ab30cbee77b59317d6e30001210237af13eb2d84e4545af287b919c2282019c9691cc509e78e196a9d8274ed1be0ffffffff0100000000000000001976a914f1b3ed2eda9a2ebe5a9374f692877cdf87c0f95b88ac00000000", "P2SH"], +["A transaction with a low-S signature."], +[[["a54b3bff74c910adff4f81b641587440c9f3b02a67c69b06d4eea36c0d199bac", 0, "DUP HASH160 0x14 0x33b9fe6942f268ae728136046e638c8863bed91f EQUALVERIFY CHECKSIG"]], +"0100000001ac9b190d6ca3eed4069bc6672ab0f3c940745841b6814fffad10c974ff3b4ba5000000006b483045022100c16257354c7ec7fedda7ce724abfc6a8f26fd433d08b1a11bd01a8f0f157b1fb02206f96be605bbab32ad0e2cde63c6a46927cee9ca4582f30a4e7b8876e80a089180121039770779101a01dbbf6c25466df8869e648dc75a51d64ebf5136c824c32fcc9c8ffffffff0100000000000000001976a914aaf0529f46a86271255621ed566e79e67345916d88ac00000000", "P2SH,LOW_S"], + +["A transaction with a high-S signature."], +[[["a54b3bff74c910adff4f81b641587440c9f3b02a67c69b06d4eea36c0d199bac", 0, "DUP HASH160 0x14 0x33b9fe6942f268ae728136046e638c8863bed91f EQUALVERIFY CHECKSIG"]], +"0100000001ac9b190d6ca3eed4069bc6672ab0f3c940745841b6814fffad10c974ff3b4ba5000000006c493046022100c16257354c7ec7fedda7ce724abfc6a8f26fd433d08b1a11bd01a8f0f157b1fb0221009069419fa4454cd52f1d3219c395b96c3dc0404257196f96d819d71e4f95b8290121039770779101a01dbbf6c25466df8869e648dc75a51d64ebf5136c824c32fcc9c8ffffffff0100000000000000001976a914aaf0529f46a86271255621ed566e79e67345916d88ac00000000", "P2SH"], + ["Make diffs cleaner by leaving a comment here without comma at the end"] ]