From 0507f6a1170611611d275874b016b23eb5bf6c68 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Fri, 12 Nov 2021 16:39:13 -0600 Subject: [PATCH] Validate message signature encoding Fixes #1069. Before this commit, a non-hex encoded pubkey sent as part of the privmsg signature raised an Exception, this commit fixes this, checking the encoding of both the pubkey and signature fields before sending them from daemon to client. --- jmdaemon/jmdaemon/message_channel.py | 27 +++++++++++++++++++++------ jmdaemon/test/test_message_channel.py | 13 ++++++++----- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/jmdaemon/jmdaemon/message_channel.py b/jmdaemon/jmdaemon/message_channel.py index 277abc09d..96be37ec6 100644 --- a/jmdaemon/jmdaemon/message_channel.py +++ b/jmdaemon/jmdaemon/message_channel.py @@ -8,6 +8,7 @@ encrypted_commands, commitment_broadcast_list, offername_list,\ fidelity_bond_cmd_list from jmbase.support import get_log +from jmbase import hextobin from functools import wraps log = get_log() @@ -918,7 +919,6 @@ def on_privmsg(self, nick, message): #Other ill formatted messages will be caught in the try block. if len(message) < 2: return - if message[0] != COMMAND_PREFIX: log.debug('message not a cmd') return @@ -926,16 +926,31 @@ def on_privmsg(self, nick, message): if cmd_string not in plaintext_commands + encrypted_commands: log.debug('cmd not in cmd_list, line="' + message + '"') return + badsigmsg = "Sig not properly appended to privmsg, ignoring" #Verify nick ownership - sig = message[1:].split(' ')[-2:] + try: + pub, sig = message[1:].split(' ')[-2:] + except Exception: + log.debug(badsigmsg) + return #reconstruct original message without cmd rawmessage = ' '.join(message[1:].split(' ')[1:-2]) - #sanity check that the sig was appended properly - if len(sig) != 2 or len(rawmessage) == 0: - log.debug("Sig not properly appended to privmsg, ignoring") + # can happen if not enough fields for command, (stuff), pub, sig: + if len(rawmessage) == 0: + log.debug(badsigmsg) + return + # Sanitising signature before attempting to verify: + # Note that the sig itself can be any garbage, because `ecdsa_verify` + # swallows any fail and returns False; but the pubkey is assumed + # to be hex-encoded, and the signature base64 encoded, so check early: + try: + dummypub = hextobin(pub) + dummysig = base64.b64decode(sig) + except Exception: + log.debug(badsigmsg) return self.daemon.request_signature_verify( - rawmessage + str(self.hostid), message, sig[1], sig[0], nick, + rawmessage + str(self.hostid), message, sig, pub, nick, NICK_HASH_LENGTH, NICK_MAX_ENCODED, str(self.hostid)) def on_verified_privmsg(self, nick, message): diff --git a/jmdaemon/test/test_message_channel.py b/jmdaemon/test/test_message_channel.py index a9a8bfbb6..d68321bcf 100644 --- a/jmdaemon/test/test_message_channel.py +++ b/jmdaemon/test/test_message_channel.py @@ -17,8 +17,11 @@ import threading import binascii import jmbitcoin as bitcoin +from jmbase import bintohex from dummy_mc import DummyMessageChannel +# note: completely invalid sig is fine, as long as it's encoded right: +dummy_sig_str = bintohex(b"\x02"*33) + " " + base64.b64encode(b"\x01"*72).decode() jlog = get_log() @@ -151,8 +154,8 @@ def test_setup_mc(): #Simulate order receipt on 2 of 3 msgchans from this nick; #note that it will have its active chan set to mc "1" because that #is the last it was seen on: - dmcs[0].on_privmsg(cp1, "!reloffer 0 4000 5000 100 0.2 abc def") - dmcs[1].on_privmsg(cp1, "!reloffer 0 4000 5000 100 0.2 abc def") + dmcs[0].on_privmsg(cp1, "!reloffer 0 4000 5000 100 0.2 " + dummy_sig_str) + dmcs[1].on_privmsg(cp1, "!reloffer 0 4000 5000 100 0.2 " + dummy_sig_str) time.sleep(0.5) #send back a response mcc.privmsg(cp1, "fill", "0") @@ -210,7 +213,7 @@ def test_setup_mc(): #first, pretend they all showed up on all 3 mcs: for m in dmcs: for cp in cps: - m.on_privmsg(cp, "!reloffer 0 400000 500000 100 0.002 abc def") + m.on_privmsg(cp, "!reloffer 0 400000 500000 100 0.002 " + dummy_sig_str) #next, call main fill function mcc.fill_orders(new_offers, 1000, "dummypubkey", "dummycommit") #now send a dummy transaction to this same set. @@ -242,7 +245,7 @@ def test_setup_mc(): #have the cps rearrive for m in dmcs: for cp in cps: - m.on_privmsg(cp, "!reloffer 0 4000 5000 100 0.2 abc def") + m.on_privmsg(cp, "!reloffer 0 4000 5000 100 0.2 " + dummy_sig_str) ##################################################################### #next series of messages are to test various normal and abnormal @@ -255,7 +258,7 @@ def test_setup_mc(): #invalid missing field dmcs[0].on_pubmsg(cps[2], "!hp2") #receive commitment via privmsg to trigger commitment_transferred - dmcs[0].on_privmsg(cps[2], "!hp2 deadbeef abc def") + dmcs[0].on_privmsg(cps[2], "!hp2 deadbeef " + dummy_sig_str) #simulate receipt of order cancellation #valid dmcs[0].on_pubmsg(cps[2], "!cancel 2")