From db458ec1cfa01502dfc3c298599610911a2fd2f9 Mon Sep 17 00:00:00 2001
From: AdamISZ <ekaggata@gmail.com>
Date: Mon, 9 Apr 2018 23:34:53 +0200
Subject: [PATCH] Correct handling of invalid, duplicate or spurious signatures
 by taker.

Signatures sent a second time now are correctly ignored, rather than prompting
a removal for the utxo entry which fails since it's already been removed.
Also corrects the case where a junk signature can trigger a crash in verify_tx_input,
if its script deserialization results in non-string entries (integer or None).
Test cases in test_taker updated to include these error cases, now passing.
---
 jmclient/jmclient/taker.py  | 20 ++++++++++++++++++--
 jmclient/test/test_taker.py |  4 ++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/jmclient/jmclient/taker.py b/jmclient/jmclient/taker.py
index a47d054..b48fb2d 100644
--- a/jmclient/jmclient/taker.py
+++ b/jmclient/jmclient/taker.py
@@ -434,6 +434,10 @@ class Taker(object):
         """
         if self.aborted:
             return False
+        if nick not in self.nonrespondants:
+            jlog.debug(('add_signature => nick={} '
+                       'not in nonrespondants {}').format(nick, self.nonrespondants))
+            return
         sig = base64.b64decode(sigb64).encode('hex')
         inserted_sig = False
         txhex = btc.serialize(self.latest_tx)
@@ -462,6 +466,11 @@ class Taker(object):
             #the segwit-style signature. Note that this allows a mixed
             #SW/non-SW transaction as each utxo is interpreted separately.
             sig_deserialized = btc.deserialize_script(sig)
+            #verify_tx_input will not even parse the script if it has integers or None,
+            #so abort in case we were given a junk sig:
+            if not all([not isinstance(x, int) and x for x in sig_deserialized]):
+                print("Junk signature: ", sig_deserialized, ", not attempting to verify")
+                break
             if len(sig_deserialized) == 2:
                 ver_sig, ver_pub = sig_deserialized
                 wit = None
@@ -470,6 +479,7 @@ class Taker(object):
             else:
                 jlog.debug("Invalid signature message - more than 3 items")
                 break
+            print("Got sig_deserialized: ", sig_deserialized)
             ver_amt = utxo_data[i]['value'] if wit else None
             sig_good = btc.verify_tx_input(txhex, u[0], utxo_data[i]['script'],
                                                ver_sig, ver_pub, witness=wit,
@@ -483,11 +493,17 @@ class Taker(object):
                     self.latest_tx["ins"][u[0]]["script"] = sig
                 inserted_sig = True
                 # check if maker has sent everything possible
-                self.utxos[nick].remove(u[1])
+                try:
+                    self.utxos[nick].remove(u[1])
+                except ValueError:
+                    pass
                 if len(self.utxos[nick]) == 0:
                     jlog.debug(('nick = {} sent all sigs, removing from '
                                'nonrespondant list').format(nick))
-                    self.nonrespondants.remove(nick)
+                    try:
+                        self.nonrespondants.remove(nick)
+                    except ValueError:
+                        pass
                 break
         if not inserted_sig:
             jlog.debug('signature did not match anything in the tx')
diff --git a/jmclient/test/test_taker.py b/jmclient/test/test_taker.py
index 62b567b..2019a24 100644
--- a/jmclient/test/test_taker.py
+++ b/jmclient/test/test_taker.py
@@ -416,8 +416,12 @@ def test_on_sig(createcmtdata, dummyaddr, signmethod, schedule):
     tx3 = bitcoin.sign(tx, 2, privs[2])
     sig3 = b64encode(bitcoin.deserialize(tx3)['ins'][2]['script'].decode('hex'))
     taker.on_sig("cp1", sig3)
+    #try sending the same sig again; should be ignored
+    taker.on_sig("cp1", sig3)
     tx4 = bitcoin.sign(tx, 3, privs[3])
     sig4 = b64encode(bitcoin.deserialize(tx4)['ins'][3]['script'].decode('hex'))
+    #try sending junk instead of cp2's correct sig
+    taker.on_sig("cp2", str("junk"))
     taker.on_sig("cp2", sig4)
     tx5 = bitcoin.sign(tx, 4, privs[4])
     #Before completing with the final signature, which will trigger our own