SFT-4502: fix p2wpkh nested in p2sh hash change validation

.gitignore

@@ -99,3 +99,4 @@ simulator/snapshots/
 *.gif
 *.pem
 .vscode
+.codex

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")

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

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

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'

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))

ports/stm32/boards/Passport/modules/tests/unit/psbt_change_validation.py

@@ -0,0 +1,128 @@
+# SPDX-FileCopyrightText: © 2026 Foundation Devices, Inc. <hello@foundation.xyz>
+#
+# 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')