Enforce the LOW_S script validation flag

bitcoinj · Oct 31, 2015 · 377d226 · 377d226
1 parent 92df23c
commit 377d226
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 4 deletions.
diff --git a/core/src/main/java/org/bitcoinj/crypto/TransactionSignature.java b/core/src/main/java/org/bitcoinj/crypto/TransactionSignature.java
@@ -156,18 +156,42 @@ public ECKey.ECDSASignature toCanonicalised() {
 
     /**
      * 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}.
      */
-    public static TransactionSignature decodeFromBitcoin(byte[] bytes, boolean requireCanonical) throws VerificationException {
+    @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 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 {
             sig = ECKey.ECDSASignature.decodeFromDER(bytes);
         } 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
@@ -1320,8 +1320,8 @@ private static void executeCheckSig(Transaction txContainingThis, int index, Scr
         // 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
@@ -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
@@ -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"]
 ]