diff --git a/shared/chains.py b/shared/chains.py index 780ce045..73dc8d4e 100644 --- a/shared/chains.py +++ b/shared/chains.py @@ -101,29 +101,51 @@ class ChainsBase: or (version == cls.slip132[addr_fmt].priv) return node + @classmethod + def script_pubkey(cls, addr_fmt, pubkey=None, script=None): + digest = None + if addr_fmt & AFC_SCRIPT: + assert script, "need witness/redeem script" + + if addr_fmt in [AF_P2WSH, AF_P2WSH_P2SH]: + digest = ngu.hash.sha256s(script) + # bech32 encoded segwit p2sh + spk = b'\x00\x20' + digest + if addr_fmt == AF_P2WSH_P2SH: + # segwit p2wsh encoded as classic P2SH + digest = hash160(spk) + spk = b'\xA9\x14' + digest + b'\x87' + + else: + assert addr_fmt == AF_P2SH + digest = hash160(script) + spk = b'\xA9\x14' + digest + b'\x87' + + else: + assert pubkey + keyhash = ngu.hash.hash160(pubkey) + if addr_fmt == AF_P2TR: + assert len(pubkey) == 32 # internal + spk = b'\x51\x20' + taptweak(pubkey) + elif addr_fmt == AF_CLASSIC: + spk = b'\x76\xA9\x14' + keyhash + b'\x88\xAC' + elif addr_fmt == AF_P2WPKH_P2SH: + redeem_script = b'\x00\x14' + keyhash + spk = b'\xA9\x14' + ngu.hash.hash160(redeem_script) + b'\x87' + elif addr_fmt == AF_P2WPKH: + spk = b'\x00\x14' + keyhash + else: + raise ValueError('bad address template: %s' % addr_fmt) + + return spk, digest + @classmethod def pubkey_to_address(cls, pubkey, addr_fmt): # - renders a pubkey to an address # - works only with single-key addresses assert not addr_fmt & AFC_SCRIPT - - if addr_fmt == AF_P2TR: - assert len(pubkey) == 32 # internal - script = b'\x51\x20' + taptweak(pubkey) - else: - keyhash = ngu.hash.hash160(pubkey) - if addr_fmt == AF_CLASSIC: - script = b'\x76\xA9\x14' + keyhash + b'\x88\xAC' - elif addr_fmt == AF_P2WPKH_P2SH: - redeem_script = b'\x00\x14' + keyhash - scripthash = ngu.hash.hash160(redeem_script) - script = b'\xA9\x14' + scripthash + b'\x87' - elif addr_fmt == AF_P2WPKH: - script = b'\x00\x14' + keyhash - else: - raise ValueError('bad address template: %s' % addr_fmt) - - return cls.render_address(script) + spk, _ = cls.script_pubkey(addr_fmt, pubkey=pubkey) + return cls.render_address(spk) @classmethod def address(cls, node, addr_fmt): diff --git a/shared/psbt.py b/shared/psbt.py index 287dbdb9..18cdbfe5 100644 --- a/shared/psbt.py +++ b/shared/psbt.py @@ -20,7 +20,7 @@ from serializations import CTxIn, CTxInWitness, CTxOut, ser_string, COutPoint from serializations import ser_sig_der, uint256_from_str, ser_push_data from serializations import SIGHASH_ALL, SIGHASH_SINGLE, SIGHASH_NONE, SIGHASH_ANYONECANPAY from serializations import ALL_SIGHASH_FLAGS, SIGHASH_DEFAULT -from opcodes import OP_CHECKMULTISIG +from opcodes import OP_CHECKMULTISIG, OP_RETURN from glob import settings from precomp_tag_hash import TAP_TWEAK_H, TAP_SIGHASH_H @@ -39,7 +39,7 @@ from public_constants import ( PSBT_IN_OUTPUT_INDEX, PSBT_IN_SEQUENCE, PSBT_IN_REQUIRED_TIME_LOCKTIME, PSBT_IN_REQUIRED_HEIGHT_LOCKTIME, MAX_SIGNERS, AF_P2WSH, AF_P2WSH_P2SH, AF_P2SH, AF_P2TR, AF_P2WPKH, AF_CLASSIC, AF_P2WPKH_P2SH, - AFC_SEGWIT, + AFC_SEGWIT, AF_BARE_PK ) psbt_tmp256 = bytearray(256) @@ -510,7 +510,7 @@ class psbtOutputProxy(psbtProxy): # - must match expected address for this output, coming from unsigned txn af, addr_or_pubkey = txo.get_address() - if (not self.sp_idxs) or (af in ["op_return", None]): + if (not self.sp_idxs) or (af in [OP_RETURN, None]): # num_ours == 0 # - not considered fraud because other signers looking at PSBT may have them # - user will see them as normal outputs, which they are from our PoV. @@ -539,7 +539,7 @@ class psbtOutputProxy(psbtProxy): af = AF_TO_STR_AF[af] raise FraudulentChangeOutput(idx, "%s change output is fraudulent\n\n%s" % (af, err)) - if af == 'p2pk': + if af == AF_BARE_PK: # output is compressed public key (not a hash, much less common) # uncompressed public keys not supported! assert len(addr_or_pubkey) == 33 @@ -796,7 +796,7 @@ class psbtInputProxy(psbtProxy): # - also validates redeem_script when present merkle_root = redeem_script = None - if self.af == "op_return": + if self.af == OP_RETURN: return if self.af is None: @@ -815,7 +815,7 @@ class psbtInputProxy(psbtProxy): self.sp_idxs = None return - if self.af == 'p2pk': + if self.af == AF_BARE_PK: # input is single compressed public key (less common) # uncompressed public keys not supported! assert len(addr_or_pubkey) == 33 @@ -1698,7 +1698,7 @@ class psbtObject(psbtProxy): assert txo.nValue >= 0, "negative output value: o%d" % idx total_out += txo.nValue - if (txo.nValue == 0) and (af != "op_return"): + if (txo.nValue == 0) and (af != OP_RETURN): # OP_RETURN outputs have nValue=0 standard zero_val_outs += 1 @@ -1748,7 +1748,7 @@ class psbtObject(psbtProxy): ) self.warnings.append(('Troublesome Change Outs', msg)) - if af == "op_return": + if af == OP_RETURN: num_op_return += 1 if len(txo.scriptPubKey) > 83: num_op_return_size += 1 diff --git a/shared/serializations.py b/shared/serializations.py index b6791dac..0fd827b3 100755 --- a/shared/serializations.py +++ b/shared/serializations.py @@ -19,7 +19,7 @@ from ubinascii import hexlify as b2a_hex import ustruct as struct import ngu from opcodes import * -from public_constants import AF_P2WPKH, AF_P2TR, AF_P2SH, AF_P2WSH, AF_CLASSIC +from public_constants import AF_CLASSIC, AF_P2WPKH, AF_P2SH, AF_P2WSH, AF_P2TR, AF_BARE_PK, AF_P2TR # single-shot hash functions sha256 = ngu.hash.sha256s @@ -238,7 +238,7 @@ def disassemble(script): # OP_0 included here #print('dis %d: opcode=%d' % (offset, c)) yield (None, c) - except Exception: + except Exception as e: # import sys;sys.print_exception(e) raise ValueError("bad script") @@ -376,14 +376,18 @@ class CTxOut(object): return AF_CLASSIC, self.scriptPubKey[3:3+20] if self.is_p2sh(): + # can be: + # * bare P2SH + # * P2SH-P2WPKH + # * P2SH-P2WSH return AF_P2SH, self.scriptPubKey[2:2+20] if self.is_p2pk(): # rare, pay to full pubkey - return 'p2pk', self.scriptPubKey[2:2+33] + return AF_BARE_PK, self.scriptPubKey[2:2+33] if self.scriptPubKey[0] == OP_RETURN: - return 'op_return', self.scriptPubKey + return OP_RETURN, self.scriptPubKey return None, self.scriptPubKey diff --git a/shared/usb.py b/shared/usb.py index da5a2cee..07c6d304 100644 --- a/shared/usb.py +++ b/shared/usb.py @@ -243,7 +243,7 @@ class USBHandler: except CCBusyError: # auth UX is doing something else resp = b'busy' - except SpendPolicyViolation: + except SpendPolicyViolation as e: resp = b'err_Spending policy in effect' except HSMDenied: resp = b'err_Not allowed in HSM mode' @@ -266,6 +266,7 @@ class USBHandler: raise exc except Exception as exc: # catch bugs and fuzzing too + # sys.print_exception(exc) if is_simulator() or is_devmode: print("USB request caused this: ", end='') sys.print_exception(exc) diff --git a/testing/test_multisig.py b/testing/test_multisig.py index 45731177..0f732fe8 100644 --- a/testing/test_multisig.py +++ b/testing/test_multisig.py @@ -926,7 +926,7 @@ def fake_ms_txn(pytestconfig): def doit(num_ins, num_outs, M, keys, fee=10000, outvals=None, inp_addr_fmt="p2wsh", outstyles=['p2pkh'], change_outputs=[], incl_xpubs=False, hack_psbt=None, hack_change_out=False, input_amount=1E8, psbt_v2=None, bip67=True, - violate_script_key_order=False, path_mapper=None, netcode="XTN"): + violate_script_key_order=False, path_mapper=None, netcode="XTN", force_outstyle=None): psbt = BasicPSBT() if psbt_v2 is None: @@ -1024,6 +1024,12 @@ def fake_ms_txn(pytestconfig): style = outstyles[i % len(outstyles)] if i in change_outputs: + # overwrite style, change can only be of THE style + if force_outstyle: + style = force_outstyle + else: + style = addr_fmt_names[inp_addr_fmt] + make_redeem_args = dict() if hack_change_out: make_redeem_args = hack_change_out(i) @@ -1039,7 +1045,7 @@ def fake_ms_txn(pytestconfig): if 'w' in style: psbt.outputs[i].witness_script = scr - if style.endswith('p2sh'): + if 'p2sh' in style: psbt.outputs[i].redeem_script = b'\0\x20' + sha256(scr).digest() elif style.endswith('sh'): psbt.outputs[i].redeem_script = scr @@ -1158,7 +1164,7 @@ def test_ms_sign_myself(M, use_regtest, make_myself_wallet, inp_af, num_ins, dev for idx in range(M): select_wallet(idx) if incl_xpubs: - clear_ms() + clear_miniscript() _, updated = try_sign(psbt, accept_ms_import=incl_xpubs) with open(f'{sim_root_dir}/debug/myself-after.psbt', 'w') as f: f.write(b64encode(updated).decode()) @@ -3062,4 +3068,199 @@ def test_originless_keys(get_cc_key, bitcoin_core_signer, bitcoind, offer_minsc_ res = wo.sendrawtransaction(tx_hex) assert len(res) == 64 # tx id + +def test_input_script_type(clear_ms, import_ms_wallet, start_sign, end_sign, cap_story, + press_cancel, settings_set, fake_ms_txn): + + def sign_check(psbt): + # start sign MUST raise scriptPubKey mismatch on inputs or change outputs + # it does not in current master + start_sign(psbt) + _, story = cap_story() + try: + end_sign() + assert False, story + except Exception as e: + assert e.args[0] == 'Coldcard Error: Unknown multisig wallet' + return + + clear_ms() + M, N = 2, 3 + wname = "bugg" + # import wallet with script type p2wsh + keys = import_ms_wallet(M, N, addr_fmt="p2wsh", name=wname, accept=True, descriptor=True) + + # create txn with p2sh inputs + # we shouldn't even recognize these input as ours + psbt = fake_ms_txn(2, 2, M, keys, inp_af=AF_P2SH, + change_outputs=[0,1]) + sign_check(psbt) + + # create txn with p2sh-p2wsh + # we shouldn't even recognize these input as ours + psbt = fake_ms_txn(2, 2, M, keys, + change_outputs=[0,1], inp_af=AF_P2WSH_P2SH) + + sign_check(psbt) + + # ============================ + + clear_ms() + # import wallet with script type p2sh-p2wsh + keys = import_ms_wallet(M, N, addr_fmt="p2sh-p2wsh", name=wname, accept=True, descriptor=True) + + # create txn with p2wsh inputs + # we shouldn't even recognize these input as ours + psbt = fake_ms_txn(2, 2, M, keys, + change_outputs=[0,1], inp_af=AF_P2WSH) + + sign_check(psbt) + + # create txn with p2sh inputs + # we shouldn't even recognize these input as ours + psbt = fake_ms_txn(2, 2, M, keys, + change_outputs=[0,1], inp_af=AF_P2SH) + + sign_check(psbt) + + # ============================ + + clear_ms() + # import wallet with script type p2sh + keys = import_ms_wallet(M, N, addr_fmt="p2sh", name=wname, accept=True, descriptor=True) + + # create txn with p2wsh inputs + # we shouldn't even recognize these input as ours + psbt = fake_ms_txn(2, 2, M, keys, + change_outputs=[0,1], inp_af=AF_P2WSH) + + sign_check(psbt) + + # create txn with p2sh-p2wsh inputs + # we shouldn't even recognize these input as ours + psbt = fake_ms_txn(2, 2, M, keys, + change_outputs=[0,1], inp_af=AF_P2WSH_P2SH) + + sign_check(psbt) + + +def test_change_output_script_type(clear_ms, import_ms_wallet, start_sign, end_sign, cap_story, + press_cancel, settings_set, fake_ms_txn): + + def sign_check(psbt): + # start sign MUST raise scriptPubKey mismatch on inputs or change outputs + # it does not in current master + start_sign(psbt) + _, story = cap_story() + assert "Change back" not in story + assert "Consolidating" not in story + assert "Sending" in story + end_sign() # must work + + clear_ms() + M, N = 2, 3 + wname = "bugg" + # import wallet with script type p2wsh + keys = import_ms_wallet(M, N, addr_fmt="p2wsh", name=wname, accept=True, descriptor=True) + + # inputs correct, change outputs wrong address format + psbt = fake_ms_txn(2, 2, M, keys, force_outstyle="p2sh", inp_af=AF_P2WSH, + change_outputs=[0,1]) + sign_check(psbt) + + psbt = fake_ms_txn(2, 2, M, keys, force_outstyle="p2sh-p2wsh", + change_outputs=[0,1], inp_af=AF_P2WSH) + + sign_check(psbt) + + # ============================ + + clear_ms() + # import wallet with script type p2sh-p2wsh + keys = import_ms_wallet(M, N, addr_fmt="p2sh-p2wsh", name=wname, accept=True, descriptor=True) + + # inputs correct, change outputs wrong address format + psbt = fake_ms_txn(2, 2, M, keys, force_outstyle="p2wsh", + change_outputs=[0,1], inp_af=AF_P2WSH_P2SH) + + sign_check(psbt) + + psbt = fake_ms_txn(2, 2, M, keys, force_outstyle="p2sh", + change_outputs=[0,1], inp_af=AF_P2WSH_P2SH) + + sign_check(psbt) + + # ============================ + + clear_ms() + M, N = 2, 3 + wname = "bugg" + # import wallet with script type p2sh + keys = import_ms_wallet(M, N, addr_fmt="p2sh", name=wname, accept=True, descriptor=True) + + # inputs correct, change outputs wrong address format + psbt = fake_ms_txn(2, 2, M, keys, force_outstyle="p2wsh", + change_outputs=[0,1], inp_af=AF_P2SH) + + sign_check(psbt) + + psbt = fake_ms_txn(2, 2, M, keys, force_outstyle="p2sh-p2wsh", + change_outputs=[0,1], inp_af=AF_P2SH, segwit_in=True) + + sign_check(psbt) + + +def test_sh_vs_wrapped_segwit_psbt(clear_ms, import_ms_wallet, start_sign, end_sign, cap_story, + press_cancel, settings_set, fake_ms_txn): + + clear_ms() + M, N = 2, 3 + wname = "spk_check_sh_shwsh" + # import wallet with script type p2sh + keys = import_ms_wallet(M, N, addr_fmt="p2sh", name=wname, accept=True, descriptor=True) + + def hack(psbt_in): + for inp in psbt_in.inputs: + # switch scripts so it looks like bare p2sh instead wrapped segwit script hash + # it even has our keys, and script is correct + inp.redeem_script = inp.witness_script + inp.witness_script = None + + # PSBT has p2sh-p2wsh inputs & outputs + # but PSBT creator made a mistake and filled redeem/witness like in p2sh (see hack) + psbt = fake_ms_txn(2, 2, M, keys, inp_af=AF_P2WSH_P2SH, hack_psbt=hack) + + start_sign(psbt) + time.sleep(.1) + title, story = cap_story() + assert "OK TO SEND?" not in title + assert "spk mismatch" in story + + +def test_wrapped_segwit_vs_sh_psbt(clear_ms, import_ms_wallet, start_sign, end_sign, cap_story, + press_cancel, settings_set, fake_ms_txn): + + clear_ms() + M, N = 2, 3 + wname = "spk_check_shwsh_sh" + # import wallet with script type p2sh-p2wsh + keys = import_ms_wallet(M, N, addr_fmt="p2sh-p2wsh", name=wname, accept=True, descriptor=True) + + def hack(psbt_in): + for inp in psbt_in.inputs: + # switch scripts so it looks like bare p2sh instead wrapped segwit script hash + # it even has our keys, and script is correct + inp.witness_script = inp.redeem_script + inp.redeem_script = b"\x00\x20" + sha256(inp.witness_script).digest() + + # PSBT has p2sh inputs & outputs + # but PSBT creator made a mistake and filled redeem/witness like in p2sh (see hack) + psbt = fake_ms_txn(2, 2, M, keys, inp_af=AF_P2SH, hack_psbt=hack) + + start_sign(psbt) + time.sleep(.1) + title, story = cap_story() + assert "OK TO SEND?" not in title + assert "spk mismatch" in story + # EOF diff --git a/testing/test_nfc.py b/testing/test_nfc.py index 35105b57..5a0ef8c8 100644 --- a/testing/test_nfc.py +++ b/testing/test_nfc.py @@ -11,7 +11,8 @@ from struct import pack, unpack import ndef from hashlib import sha256 from txn import * -from charcodes import KEY_NFC, KEY_QR +from constants import unmap_addr_fmt +from charcodes import KEY_NFC @pytest.mark.parametrize('case', range(6)) diff --git a/testing/test_teleport.py b/testing/test_teleport.py index 7a0b9c2d..2b5cbad4 100644 --- a/testing/test_teleport.py +++ b/testing/test_teleport.py @@ -429,7 +429,7 @@ def test_teleport_ms_sign(M, use_regtest, make_myself_wallet, segwit, num_ins, d fake_ms_txn, try_sign, incl_xpubs, bitcoind, cap_story, need_keypress, cap_menu, pick_menu_item, grab_payload, rx_complete, press_select, ndef_parse_txn_psbt, press_nfc, nfc_read, settings_get, settings_set, - txid_from_export_prompt, sim_root_dir, + txid_from_export_prompt, sim_root_dir, goto_home, set_hobble, hobbled, readback_bbqr, nfc_is_enabled): # IMPORTANT: won't work if you start simulator with --ms flag. Use no args @@ -445,6 +445,10 @@ def test_teleport_ms_sign(M, use_regtest, make_myself_wallet, segwit, num_ins, d N = len(keys) assert M<=N + if hobbled: + set_hobble(True, {'okeys'}) + goto_home() + psbt = fake_ms_txn(15, num_outs, M, keys, inp_addr_fmt=af, incl_xpubs=incl_xpubs, outstyles=["p2sh-p2wsh", af, af, af], change_outputs=list(range(1,num_outs)))