Fix OP_CHECKMULTISIG silently accepting a non-matching signature

buidl/op.py

@@ -833,6 +833,13 @@ def op_checkmultisig(stack, tx_obj, input_index):
                 if point.verify(z, sig):
                     # break if so, this sig is valid!
                     break
+            else:
+                # exhausted every remaining pubkey without a match --
+                # this signature does not correspond to any of them,
+                # so the whole multisig check must fail. Without this,
+                # a signature that doesn't verify against any pubkey
+                # was silently treated as optional instead of invalid.
+                return False
         # if we made it this far, we have to add a 1 to the stack
         # use encode_num(1)
         stack.append(encode_num(1))

buidl/test/test_op.py

@@ -563,6 +563,34 @@ def test_op_checkmultisig(self):
         self.assertTrue(op_checkmultisig(stack, tx_obj, 0))
         self.assertEqual(decode_num(stack[0]), 1)
 
+    def test_op_checkmultisig_rejects_signature_that_matches_no_pubkey(self):
+        # Same real 2-of-2 spend as test_op_checkmultisig above, but with
+        # the first signature replaced by a real, validly-DER-encoded
+        # signature from a completely unrelated transaction (the first
+        # tuple in test_op_checksig above) -- i.e. one that corresponds
+        # to neither sec1 nor sec2. Before this fix, op_checkmultisig's
+        # inner `while points:` loop had no way to detect "exhausted
+        # every pubkey without a match", so a non-matching signature was
+        # silently treated as if it were optional and this still
+        # returned True. A 2-of-2 multisig must never be satisfiable by
+        # fewer than 2 real, matching signatures.
+        raw_tx = "0100000001868278ed6ddfb6c1ed3ad5f8181eb0c7a385aa0836f01d5e4789e6bd304d87221a000000db00483045022100dc92655fe37036f47756db8102e0d7d5e28b3beb83a8fef4f5dc0559bddfb94e02205a36d4e4e6c7fcd16658c50783e00c341609977aed3ad00937bf4ee942a8993701483045022100da6bee3c93766232079a01639d07fa869598749729ae323eab8eef53577d611b02207bef15429dcadce2121ea07f233115c6f09034c0be68db99980b9a6c5e75402201475221022626e955ea6ea6d98850c994f9107b036b1334f18ca8830bfff1295d21cfdb702103b287eaf122eea69030a0e9feed096bed8045c8b98bec453e1ffac7fbdbd4bb7152aeffffffff04d3b11400000000001976a914904a49878c0adfc3aa05de7afad2cc15f483a56a88ac7f400900000000001976a914418327e3f3dda4cf5b9089325a4b95abdfa0334088ac722c0c00000000001976a914ba35042cfe9fc66fd35ac2224eebdafd1028ad2788acdc4ace020000000017a91474d691da1574e6b3c192ecfb52cc8984ee7b6c568700000000"
+        tx_obj = Tx.parse(BytesIO(bytes.fromhex(raw_tx)))
+        unrelated_sig = bytes.fromhex(
+            "30440220636b9f822ea2f85e6375ecd066a49cc74c20ec4f7cf0485bebe6cc68da92d8ce022068ae17620b12d99353287d6224740b585ff89024370a3212b583fb454dce7c1601"
+        )
+        sig2 = bytes.fromhex(
+            "3045022100da6bee3c93766232079a01639d07fa869598749729ae323eab8eef53577d611b02207bef15429dcadce2121ea07f233115c6f09034c0be68db99980b9a6c5e75402201"
+        )
+        sec1 = bytes.fromhex(
+            "022626e955ea6ea6d98850c994f9107b036b1334f18ca8830bfff1295d21cfdb70"
+        )
+        sec2 = bytes.fromhex(
+            "03b287eaf122eea69030a0e9feed096bed8045c8b98bec453e1ffac7fbdbd4bb71"
+        )
+        stack = [b"", unrelated_sig, sig2, b"\x02", sec1, sec2, b"\x02"]
+        self.assertFalse(op_checkmultisig(stack, tx_obj, 0))
+
     def test_op_cltv(self):
         locktime_0 = Locktime(1234)
         locktime_1 = Locktime(2345)