diff --git a/.gitignore b/.gitignore index b79c1e9c5..b305998b5 100644 --- a/.gitignore +++ b/.gitignore @@ -99,3 +99,4 @@ simulator/snapshots/ *.gif *.pem .vscode +.codex diff --git a/Justfile b/Justfile index 8c483b1c3..fb58cf522 100644 --- a/Justfile +++ b/Justfile @@ -71,8 +71,19 @@ sim screen="mono" ext="": # Run unit tests. test: - just simulator/build color - cd ports/stm32/boards/Passport/modules/tests; python3 -m pytest . --simulatordir=$(pwd)/simulator + cd "{{justfile_directory()}}/ports/stm32/boards/Passport/modules/tests" && \ + python3 -m pytest . --simulatordir="{{justfile_directory()}}/simulator" + +# Run simulator-backed Passport module unit tests from the repo root. +unit-test filter="": + #!/usr/bin/env bash + set -euo pipefail + cd "{{justfile_directory()}}/ports/stm32/boards/Passport/modules/tests" + extra_args="" + if [ -n "{{filter}}" ]; then + extra_args="-k {{filter}}" + fi + python3 -m pytest test_unit.py -vv -s --simulatordir="{{justfile_directory()}}/simulator" ${extra_args} # Lint the codebase. lint: (run-in-docker "just ports/stm32/lint") (run-in-docker "just extmod/foundation-rust/lint") diff --git a/flake.nix b/flake.nix index 6abed21b2..700a53a0d 100644 --- a/flake.nix +++ b/flake.nix @@ -108,6 +108,19 @@ } ); + python-with-packages = pkgs.python313.withPackages (ps: with ps; [ + opencv4.override { + enableGtk2 = true; + gtk2 = pkgs.gtk2; + } + pillow + pysdl2 + pytest + pip + virtualenv + autopep8 + ]); + buildPackages = with pkgs; [ @@ -124,10 +137,7 @@ libusb1 openssl pkg-config - python3 - python3Packages.pip - python3Packages.virtualenv - python3Packages.autopep8 + python-with-packages reuse rust-cbindgen xterm diff --git a/ports/stm32/boards/Passport/modules/psbt.py b/ports/stm32/boards/Passport/modules/psbt.py index d7032566c..9bf8d37e3 100644 --- a/ports/stm32/boards/Passport/modules/psbt.py +++ b/ports/stm32/boards/Passport/modules/psbt.py @@ -53,6 +53,26 @@ def purpose_mismatch_allowed(purpose): return (purpose & 0x7fffffff) in [84, 86] +def expected_single_sig_addr_type(subpath): + # Map the derivation purpose in a single-sig BIP32 path to the script family + # the change output must use. + if not subpath or len(subpath) < 6: + return None + + purpose = subpath[1] & 0x7fffffff + + if purpose == 44: + return 'p2pkh' + if purpose == 49: + return 'p2sh-p2wpkh' + if purpose == 84: + return 'p2wpkh' + if purpose == 86: + return 'p2tr' + + return None + + def _skip_n_objs(fd, n, cls): # skip N sized objects in the stream, for example a vectors of CTxIns # - returns starting position @@ -409,11 +429,14 @@ def validate(self, out_idx, txo, my_xfp, active_multisig): if self.subpaths and len(self.subpaths) == 1: # p2pk, p2pkh, p2wpkh cases expect_pubkey, = self.subpaths.keys() + expected_addr_type = expected_single_sig_addr_type(next(iter(self.subpaths.values()))) elif self.tap_subpaths and len(self.tap_subpaths) == 1: expect_pubkey, = self.tap_subpaths.keys() + expected_addr_type = 'p2tr' else: # p2wsh/p2sh cases need full set of pubkeys, and therefore redeem script expect_pubkey = None + expected_addr_type = None if addr_type == 'p2pk': # output is public key (not a hash, much less common) @@ -445,8 +468,15 @@ def validate(self, out_idx, txo, my_xfp, active_multisig): redeem_script[0] == 0 and redeem_script[1] == 20: # it's actually segwit p2pkh inside p2sh - pkh = redeem_script[2:22] - expect_pkh = hash160(expect_pubkey) + if expected_addr_type and expected_addr_type != 'p2sh-p2wpkh': + raise FraudulentChangeOutput(out_idx, "Change output uses the wrong script type") + + expect_redeem_script = b'\x00\x14' + hash160(expect_pubkey) + if redeem_script != expect_redeem_script: + raise FraudulentChangeOutput(out_idx, + "P2SH-P2WPKH redeem script provided, and doesn't match") + + expect_pkh = hash160(expect_redeem_script) else: # Multisig change output, for wallet we're supposed to be a part of. @@ -504,8 +534,15 @@ def validate(self, out_idx, txo, my_xfp, active_multisig): elif addr_type == 'p2pkh': # input is hash160 of a single public key assert len(addr_or_pubkey) == 20 + + actual_addr_type = 'p2wpkh' if is_segwit else 'p2pkh' + if expected_addr_type and actual_addr_type != expected_addr_type: + raise FraudulentChangeOutput(out_idx, "Change output uses the wrong script type") + expect_pkh = hash160(expect_pubkey) elif addr_type == 'p2tr': + if expected_addr_type and expected_addr_type != 'p2tr': + raise FraudulentChangeOutput(out_idx, "Change output uses the wrong script type") expect_pkh = output_script(expect_pubkey, None)[2:] else: # we don't know how to "solve" this type of input diff --git a/ports/stm32/boards/Passport/modules/tests/test_unit.py b/ports/stm32/boards/Passport/modules/tests/test_unit.py index a088f75fb..6b6041d84 100644 --- a/ports/stm32/boards/Passport/modules/tests/test_unit.py +++ b/ports/stm32/boards/Passport/modules/tests/test_unit.py @@ -26,3 +26,7 @@ def test_ui(test): def test_foundation(test): assert test('foundation.py') == b'OK' + + +def test_psbt_change_validation(test): + assert test('psbt_change_validation.py') == b'OK' diff --git a/ports/stm32/boards/Passport/modules/tests/unit/foundation.py b/ports/stm32/boards/Passport/modules/tests/unit/foundation.py index 9ab69a43f..569e636d4 100644 --- a/ports/stm32/boards/Passport/modules/tests/unit/foundation.py +++ b/ports/stm32/boards/Passport/modules/tests/unit/foundation.py @@ -23,7 +23,9 @@ def should_fail(f): should_fail(lambda: foundation.qr.init()) should_fail(lambda: foundation.qr.init(None, None, None)) -foundation.qr.init(HOR_RES, VER_RES, bytearray(HOR_RES * VER_RES)) +should_fail(lambda: foundation.qr.init(HOR_RES, VER_RES, bytearray(HOR_RES * VER_RES))) +foundation.qr.init(HOR_RES, VER_RES) +assert len(foundation.qr.framebuffer) >= HOR_RES * VER_RES should_fail(lambda: foundation.convert_rgb565_to_grayscale()) should_fail(lambda: foundation.convert_rgb565_to_grayscale(None, None, None, None)) diff --git a/ports/stm32/boards/Passport/modules/tests/unit/psbt_change_validation.py b/ports/stm32/boards/Passport/modules/tests/unit/psbt_change_validation.py new file mode 100644 index 000000000..d5312df61 --- /dev/null +++ b/ports/stm32/boards/Passport/modules/tests/unit/psbt_change_validation.py @@ -0,0 +1,128 @@ +# SPDX-FileCopyrightText: © 2026 Foundation Devices, Inc. +# +# SPDX-License-Identifier: GPL-3.0-or-later +# +# Regression tests for PSBT change classification edge-cases. + +from exceptions import FraudulentChangeOutput +from psbt import psbtObject, psbtOutputProxy +from serializations import CTxOut, hash160 +from taproot import output_script + + +MY_XFP = 0x12345678 +PURPOSE_49 = 0x80000000 | 49 +PURPOSE_84 = 0x80000000 | 84 +PURPOSE_86 = 0x80000000 | 86 +COIN_0 = 0x80000000 +ACCOUNT_0 = 0x80000000 +PUBKEY = b'\x02' + (b'\x11' * 32) +TAP_PUBKEY = b'\x33' * 32 +PUBKEY_HASH = hash160(PUBKEY) +REDEEM_SCRIPT = b'\x00\x14' + PUBKEY_HASH +GOOD_P2SH = b'\xa9\x14' + hash160(REDEEM_SCRIPT) + b'\x87' +BAD_P2SH = b'\xa9\x14' + (b'\x22' * 20) + b'\x87' +NATIVE_P2WPKH = b'\x00\x14' + PUBKEY_HASH +TAPROOT_SCRIPT = output_script(TAP_PUBKEY, None) +BIP49_SUBPATH = [MY_XFP, PURPOSE_49, COIN_0, ACCOUNT_0, 1, 7] +BIP84_INPUT_SUBPATH = [MY_XFP, PURPOSE_84, COIN_0, ACCOUNT_0, 0, 3] +BIP84_CHANGE_SUBPATH = [MY_XFP, PURPOSE_84, COIN_0, ACCOUNT_0, 1, 7] +BIP86_INPUT_SUBPATH = [MY_XFP, PURPOSE_86, COIN_0, ACCOUNT_0, 0, 9] +BIP86_CHANGE_SUBPATH = [MY_XFP, PURPOSE_86, COIN_0, ACCOUNT_0, 1, 8] + + +class FakeOutput: + validate = psbtOutputProxy.validate + + def __init__(self, script_pubkey, subpaths=None, tap_subpaths=None, redeem_script=None): + self.subpaths = subpaths + self.tap_subpaths = tap_subpaths + self.redeem_script = redeem_script + self.witness_script = None + self.is_change = False + self._txo = CTxOut(0, script_pubkey) + + def parse_subpaths(self, my_xfp): + assert my_xfp == MY_XFP + return 1 + + def get(self, value): + return value + + +def must_fail(script_pubkey): + try: + FakeOutput(script_pubkey, + subpaths={PUBKEY: BIP49_SUBPATH}, + redeem_script=REDEEM_SCRIPT).validate(0, CTxOut(0, script_pubkey), MY_XFP, None) + except FraudulentChangeOutput: + return + + raise RuntimeError('expected FraudulentChangeOutput') + + +class FakeInput: + def __init__(self, subpaths=None, tap_subpaths=None, required_key=None): + self.subpaths = subpaths or {} + self.tap_subpaths = tap_subpaths or {} + self.required_key = required_key + self.fully_signed = False + + +class FakePsbt: + consider_dangerous_change = psbtObject.consider_dangerous_change + + def __init__(self, inputs, outputs): + self.inputs = inputs + self.outputs = outputs + self.warnings = [] + + +def assert_no_mixed_change_warning(outputs): + mixed_inputs = [ + FakeInput(subpaths={PUBKEY: BIP84_INPUT_SUBPATH}, required_key=PUBKEY), + FakeInput(tap_subpaths={TAP_PUBKEY: (BIP86_INPUT_SUBPATH, [])}, required_key=TAP_PUBKEY), + ] + fake_psbt = FakePsbt(mixed_inputs, outputs) + fake_psbt.consider_dangerous_change(MY_XFP) + assert fake_psbt.warnings == [] + + +valid = FakeOutput(GOOD_P2SH, + subpaths={PUBKEY: BIP49_SUBPATH}, + redeem_script=REDEEM_SCRIPT) +valid.validate(0, CTxOut(0, GOOD_P2SH), MY_XFP, None) +assert valid.is_change is True + +must_fail(BAD_P2SH) +must_fail(NATIVE_P2WPKH) + +valid_mixed_segwit_change = FakeOutput(NATIVE_P2WPKH, subpaths={PUBKEY: BIP84_CHANGE_SUBPATH}) +valid_mixed_segwit_change.validate(0, CTxOut(0, NATIVE_P2WPKH), MY_XFP, None) +assert valid_mixed_segwit_change.is_change is True +assert_no_mixed_change_warning([valid_mixed_segwit_change]) + +valid_mixed_taproot_change = FakeOutput(TAPROOT_SCRIPT, + tap_subpaths={TAP_PUBKEY: (BIP86_CHANGE_SUBPATH, [])}) +valid_mixed_taproot_change.validate(0, CTxOut(0, TAPROOT_SCRIPT), MY_XFP, None) +assert valid_mixed_taproot_change.is_change is True +assert_no_mixed_change_warning([valid_mixed_taproot_change]) + +wrong_tap_metadata_for_segwit = FakeOutput(NATIVE_P2WPKH, + tap_subpaths={TAP_PUBKEY: (BIP86_CHANGE_SUBPATH, [])}) +try: + wrong_tap_metadata_for_segwit.validate(0, CTxOut(0, NATIVE_P2WPKH), MY_XFP, None) +except FraudulentChangeOutput: + pass +else: + raise RuntimeError('expected FraudulentChangeOutput for segwit output with taproot metadata') + +wrong_segwit_metadata_for_taproot = FakeOutput(TAPROOT_SCRIPT, subpaths={PUBKEY: BIP84_CHANGE_SUBPATH}) +try: + wrong_segwit_metadata_for_taproot.validate(0, CTxOut(0, TAPROOT_SCRIPT), MY_XFP, None) +except FraudulentChangeOutput: + pass +else: + raise RuntimeError('expected FraudulentChangeOutput for taproot output with segwit metadata') + +return_value.write(b'OK')