Merge pull request #640 from Foundation-Devices/SFT-4502-fix-p2wpkh-nested-in-p2sh-hash-change-validation

### ports/stm32/boards/Passport/modules/exceptions.py
@@ -18,9 +18,18 @@ class FatalPSBTIssue(RuntimeError):
     pass
 
 
+CHANGE_ADDRESS_NOT_OURS = (
+    "Transaction rejected. The change address doesn't belong to this wallet.")
+CHANGE_MULTISIG_SETUP_MISMATCH = (
+    "Transaction rejected. The change address doesn't match this multisig wallet's setup.")
+CHANGE_WRONG_ACCOUNT_TYPE = (
+    "Transaction rejected. The change address is the wrong type for this account.")
+
+
 class FraudulentChangeOutput(FatalPSBTIssue):
     def __init__(self, out_idx, msg):
-        super().__init__('Output #%d: %s' % (out_idx, msg))
+        self.out_idx = out_idx
+        super().__init__(msg)
 
 
 class IncorrectUTXOAmount(FatalPSBTIssue):

### ports/stm32/boards/Passport/modules/psbt.py
@@ -17,9 +17,14 @@
 import sys
 from sffile import SizerFile
 from passport import mem
-from public_constants import MAX_MONEY, MAX_SIGNERS
+from public_constants import (
+    MAX_MONEY, MAX_SIGNERS, AF_CLASSIC, AF_P2WPKH_P2SH, AF_P2WPKH, AF_P2TR
+)
 from multisig_wallet import MultisigWallet, disassemble_multisig_mn
-from exceptions import FatalPSBTIssue, FraudulentChangeOutput
+from wallets.utils import get_addr_type_from_bip_numbers
+from exceptions import (FatalPSBTIssue, FraudulentChangeOutput,
+                        CHANGE_ADDRESS_NOT_OURS, CHANGE_MULTISIG_SETUP_MISMATCH,
+                        CHANGE_WRONG_ACCOUNT_TYPE)
 from serializations import ser_compact_size, deser_compact_size, hash160, deser_compact_size_bytes
 from serializations import CTxIn, CTxInWitness, CTxOut, SIGHASH_ALL, VALID_SIGHASHES, SIGHASH_DEFAULT
 from serializations import ser_push_data, uint256_from_bytes
@@ -53,6 +58,18 @@ def purpose_mismatch_allowed(purpose):
     return (purpose & 0x7fffffff) in [84, 86]
 
 
+def expected_single_sig_addr_format(subpath):
+    # Only standard full single-sig derivations have a script-family policy.
+    # Custom and incomplete paths remain visible outputs rather than aborting a
+    # signing operation merely because they cannot be classified as change.
+    # BIP45/BIP48 intentionally return None: those paths are validated against
+    # the registered script wallet below.
+    if not subpath or len(subpath) < 6:
+        return None
+
+    return get_addr_type_from_bip_numbers(subpath[1] & 0x7fffffff)
+
+
 def _skip_n_objs(fd, n, cls):
     # skip N sized objects in the stream, for example a vectors of CTxIns
     # - returns starting position
@@ -394,7 +411,8 @@ def validate(self, out_idx, txo, my_xfp, active_multisig):
         # careful and fully validate all the details.
         # - no output info is needed, in general, so
         #   any output info provided better be right, or fail as "fraud"
-        # - the signing flow derives and validates change public keys before review.
+        # - before transaction review, double_check_psbt_change_task derives
+        #   every output classified as change and validates its public key.
         # - we raise fraud alarms, since these are not innocent errors
         #
 
@@ -411,20 +429,35 @@ 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()
+            single_key_path = next(iter(self.subpaths.values()))
+            expected_addr_format = expected_single_sig_addr_format(single_key_path)
+            is_script_wallet_path = len(single_key_path) >= 2 and \
+                (single_key_path[1] & 0x7fffffff) in (45, 48)
         elif self.tap_subpaths and len(self.tap_subpaths) == 1:
             expect_pubkey, = self.tap_subpaths.keys()
+            tap_path, _ = next(iter(self.tap_subpaths.values()))
+            expected_addr_format = expected_single_sig_addr_format(tap_path)
+            is_script_wallet_path = False
         else:
             # p2wsh/p2sh cases need full set of pubkeys, and therefore redeem script
             expect_pubkey = None
+            expected_addr_format = None
+            is_script_wallet_path = False
+
+        if expect_pubkey and not expected_addr_format:
+            if active_multisig and addr_type == 'p2sh' and is_script_wallet_path:
+                # A registered 1-of-1 BIP45/BIP48 wallet can have only one of
+                # our derivation entries. Let its script validate below.
+                expect_pubkey = None
+            else:
+                # We cannot determine a script-family policy for this metadata.
+                # Do not hide the output as change, but preserve compatibility with
+                # custom derivation schemes by presenting it to the user.
+                return
 
         if addr_type == 'p2pk':
-            # output is public key (not a hash, much less common)
-            assert len(addr_or_pubkey) == 33
-
-            if addr_or_pubkey != expect_pubkey:
-                raise FraudulentChangeOutput(out_idx, "P2PK change output is fraudulent")
-
-            self.is_change = True
+            # Raw P2PK does not have a supported single-sig derivation policy.
+            # Keep it visible instead of classifying it as change.
             return
 
         # Figure out what the hashed addr should be
@@ -447,8 +480,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 expect_pubkey is None or \
+                        (expected_addr_format and expected_addr_format != AF_P2WPKH_P2SH):
+                    raise FraudulentChangeOutput(out_idx, CHANGE_WRONG_ACCOUNT_TYPE)
+
+                expect_redeem_script = b'\x00\x14' + hash160(expect_pubkey)
+                if redeem_script != expect_redeem_script:
+                    raise FraudulentChangeOutput(out_idx, CHANGE_ADDRESS_NOT_OURS)
+
+                expect_pkh = hash160(expect_redeem_script)
 
             else:
                 # Multisig change output, for wallet we're supposed to be a part of.
@@ -473,17 +513,16 @@ def validate(self, out_idx, txo, my_xfp, active_multisig):
                 try:
                     active_multisig.validate_script(witness_script or redeem_script,
                                                     subpaths=self.subpaths)
-                except BaseException as exc:
-                    raise FraudulentChangeOutput(out_idx,
-                                                 "P2WSH or P2SH change output script: %s" % exc)
+                except BaseException:
+                    raise FraudulentChangeOutput(out_idx, CHANGE_MULTISIG_SETUP_MISMATCH)
 
                 if is_segwit:
                     # p2wsh case
                     # - need witness script and check it's hash against proposed p2wsh value
                     assert len(addr_or_pubkey) == 32
                     expect_wsh = trezorcrypto.sha256(witness_script).digest()
                     if expect_wsh != addr_or_pubkey:
-                        raise FraudulentChangeOutput(out_idx, "P2WSH witness script has wrong hash")
+                        raise FraudulentChangeOutput(out_idx, CHANGE_ADDRESS_NOT_OURS)
 
                     self.is_change = True
                     return
@@ -495,8 +534,7 @@ def validate(self, out_idx, txo, my_xfp, active_multisig):
                     if redeem_script and expect_rs != redeem_script:
                         # iff they provide a redeeem script, then it needs to match
                         # what we expect it to be
-                        raise FraudulentChangeOutput(out_idx,
-                                                     "P2SH-P2WSH redeem script provided, and doesn't match")
+                        raise FraudulentChangeOutput(out_idx, CHANGE_ADDRESS_NOT_OURS)
 
                     expect_pkh = hash160(expect_rs)
                 else:
@@ -506,15 +544,22 @@ 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_format = AF_P2WPKH if is_segwit else AF_CLASSIC
+            if expected_addr_format and actual_addr_format != expected_addr_format:
+                raise FraudulentChangeOutput(out_idx, CHANGE_WRONG_ACCOUNT_TYPE)
+
             expect_pkh = hash160(expect_pubkey)
         elif addr_type == 'p2tr':
+            if expected_addr_format and expected_addr_format != AF_P2TR:
+                raise FraudulentChangeOutput(out_idx, CHANGE_WRONG_ACCOUNT_TYPE)
             expect_pkh = output_script(expect_pubkey, None)[2:]
         else:
             # we don't know how to "solve" this type of input
             return
 
         if pkh != expect_pkh:
-            raise FraudulentChangeOutput(out_idx, "Change output is fraudulent")
+            raise FraudulentChangeOutput(out_idx, CHANGE_ADDRESS_NOT_OURS)
 
         # We will check pubkey value at the last second, during signing.
         self.is_change = True

### ports/stm32/boards/Passport/modules/tasks/double_check_psbt_change_task.py
@@ -14,6 +14,7 @@ async def double_check_psbt_change_task(on_done, psbt):
     import stash
     from utils import swab32, keypath_to_str
     from errors import Error
+    from exceptions import CHANGE_ADDRESS_NOT_OURS
 
     with stash.SensitiveValues() as sv:
         # Double check the change outputs are right. This is slow, but critical because
@@ -55,9 +56,7 @@ async def double_check_psbt_change_task(on_done, psbt):
 
                 if not good:
                     # print('double_check_psbt_change_task() Fraudulent Change Error')
-                    await on_done('Transaction rejected. Passport could not verify that the change address '
-                                  'belongs to this wallet.',
-                                  Error.PSBT_FRAUDULENT_CHANGE_ERROR)
+                    await on_done(CHANGE_ADDRESS_NOT_OURS, Error.PSBT_FRAUDULENT_CHANGE_ERROR)
                     return
 
     # print('double_check_psbt_change_task() OK')

### ports/stm32/boards/Passport/modules/tests/test_unit.py
@@ -134,3 +134,7 @@ def test_restore_backup(test):
 
 def test_bip322(test):
     assert test('bip322.py') == b'OK'
+
+
+def test_psbt_change_validation(test):
+    assert test('psbt_change_validation.py') == b'OK'

### ports/stm32/boards/Passport/modules/tests/unit/psbt_change_validation.py
@@ -0,0 +1,212 @@
+# 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, CHANGE_ADDRESS_NOT_OURS,
+                        CHANGE_MULTISIG_SETUP_MISMATCH, CHANGE_WRONG_ACCOUNT_TYPE)
+from psbt import psbtObject, psbtOutputProxy
+from serializations import CTxOut, hash160
+from taproot import output_script
+import trezorcrypto
+
+
+MY_XFP = 0x12345678
+PURPOSE_49 = 0x80000000 | 49
+PURPOSE_48 = 0x80000000 | 48
+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)
+MULTISIG_SCRIPT = b'\x51\x21' + PUBKEY + b'\x51\xae'
+MULTISIG_SCRIPT_HASH = trezorcrypto.sha256(MULTISIG_SCRIPT).digest()
+P2PK_SCRIPT = b'\x21' + PUBKEY + b'\xac'
+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
+LEGACY_P2SH = b'\xa9\x14' + hash160(MULTISIG_SCRIPT) + b'\x87'
+P2WSH_REDEEM_SCRIPT = b'\x00\x20' + MULTISIG_SCRIPT_HASH
+NESTED_P2WSH = b'\xa9\x14' + hash160(P2WSH_REDEEM_SCRIPT) + b'\x87'
+NATIVE_P2WSH = b'\x00\x20' + MULTISIG_SCRIPT_HASH
+TAPROOT_SCRIPT = output_script(TAP_PUBKEY, None)
+BIP49_SUBPATH = [MY_XFP, PURPOSE_49, COIN_0, ACCOUNT_0, 1, 7]
+BIP48_SUBPATH = [MY_XFP, PURPOSE_48, COIN_0, ACCOUNT_0, 0x80000000 | 2, 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]
+BIP84_SHORT_SUBPATH = [MY_XFP, PURPOSE_84]
+BIP_UNKNOWN_SUBPATH = [MY_XFP, 0x80000000 | 123, COIN_0, ACCOUNT_0, 1, 7]
+
+
+class MockOutput:
+    validate = psbtOutputProxy.validate
+
+    def __init__(self, script_pubkey, subpaths=None, tap_subpaths=None, redeem_script=None,
+                 witness_script=None):
+        self.subpaths = subpaths
+        self.tap_subpaths = tap_subpaths
+        self.redeem_script = redeem_script
+        self.witness_script = witness_script
+        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, expected_message):
+    try:
+        MockOutput(script_pubkey,
+                   subpaths={PUBKEY: BIP49_SUBPATH},
+                   redeem_script=REDEEM_SCRIPT).validate(0, CTxOut(0, script_pubkey), MY_XFP, None)
+    except FraudulentChangeOutput as exc:
+        assert exc.args[0] == expected_message
+        return
+
+    raise RuntimeError('expected FraudulentChangeOutput')
+
+
+def validate_must_fail(output, expected_message, active_multisig=None):
+    try:
+        output.validate(0, output._txo, MY_XFP, active_multisig)
+    except FraudulentChangeOutput as exc:
+        assert exc.args[0] == expected_message
+        return
+
+    raise RuntimeError('expected FraudulentChangeOutput')
+
+
+class MockInput:
+    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 MockPsbt:
+    consider_dangerous_change = psbtObject.consider_dangerous_change
+
+    def __init__(self, inputs, outputs):
+        self.inputs = inputs
+        self.outputs = outputs
+        self.warnings = []
+
+
+class OneOfOneMultisig:
+    def validate_script(self, script, subpaths):
+        assert script == MULTISIG_SCRIPT
+        assert subpaths == {PUBKEY: BIP48_SUBPATH}
+
+
+class MismatchedMultisig:
+    def validate_script(self, script, subpaths):
+        raise ValueError('wrong M/N')
+
+
+def assert_no_change_warning(inputs, outputs):
+    mock_psbt = MockPsbt(inputs, outputs)
+    mock_psbt.consider_dangerous_change(MY_XFP)
+    assert mock_psbt.warnings == []
+
+
+segwit_inputs = [MockInput(subpaths={PUBKEY: BIP84_INPUT_SUBPATH}, required_key=PUBKEY)]
+taproot_inputs = [MockInput(tap_subpaths={TAP_PUBKEY: (BIP86_INPUT_SUBPATH, [])},
+                            required_key=TAP_PUBKEY)]
+mixed_inputs = segwit_inputs + taproot_inputs
+
+
+valid = MockOutput(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, CHANGE_ADDRESS_NOT_OURS)
+must_fail(NATIVE_P2WPKH, CHANGE_WRONG_ACCOUNT_TYPE)
+
+# Raw P2PK outputs and unknown derivations remain visible rather than being
+# treated as change or aborting a signing operation.
+raw_p2pk = MockOutput(P2PK_SCRIPT, subpaths={PUBKEY: BIP84_CHANGE_SUBPATH})
+raw_p2pk.validate(0, raw_p2pk._txo, MY_XFP, None)
+assert raw_p2pk.is_change is False
+
+# Taproot metadata is only valid for a BIP86-derived P2TR output.
+validate_must_fail(MockOutput(TAPROOT_SCRIPT,
+                              tap_subpaths={TAP_PUBKEY: (BIP84_CHANGE_SUBPATH, [])}),
+                   CHANGE_WRONG_ACCOUNT_TYPE)
+
+# A single-sig path without a recognized full account derivation is not safe to
+# classify as change, but should not prevent signing.
+for path in (BIP84_SHORT_SUBPATH, BIP_UNKNOWN_SUBPATH):
+    unsupported_path = MockOutput(NATIVE_P2WPKH, subpaths={PUBKEY: path})
+    unsupported_path.validate(0, unsupported_path._txo, MY_XFP, None)
+    assert unsupported_path.is_change is False
+
+
+# BIP48 1-of-1 script wallets have one of our derivation entries, but still
+# require registered-wallet script validation rather than the single-sig path.
+for script_pubkey, redeem_script, witness_script in (
+        (LEGACY_P2SH, MULTISIG_SCRIPT, None),
+        (NESTED_P2WSH, P2WSH_REDEEM_SCRIPT, MULTISIG_SCRIPT),
+        (NATIVE_P2WSH, None, MULTISIG_SCRIPT)):
+    one_of_one_change = MockOutput(script_pubkey,
+                                   subpaths={PUBKEY: BIP48_SUBPATH},
+                                   redeem_script=redeem_script,
+                                   witness_script=witness_script)
+    one_of_one_change.validate(0, one_of_one_change._txo, MY_XFP, OneOfOneMultisig())
+    assert one_of_one_change.is_change is True
+
+
+# A script-wallet path must not use the single-sig nested-P2WPKH branch.
+# Reject it as fraud rather than hashing the absent single-sig public key.
+for purpose in (45, 48):
+    script_wallet_path = list(BIP48_SUBPATH)
+    script_wallet_path[1] = 0x80000000 | purpose
+    validate_must_fail(MockOutput(GOOD_P2SH,
+                                  subpaths={PUBKEY: script_wallet_path},
+                                  redeem_script=REDEEM_SCRIPT),
+                       CHANGE_WRONG_ACCOUNT_TYPE,
+                       active_multisig=OneOfOneMultisig())
+
+
+validate_must_fail(MockOutput(LEGACY_P2SH,
+                              subpaths={PUBKEY: BIP48_SUBPATH},
+                              redeem_script=MULTISIG_SCRIPT),
+                   CHANGE_MULTISIG_SETUP_MISMATCH,
+                   active_multisig=MismatchedMultisig())
+
+
+valid_mixed_segwit_change = MockOutput(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
+for inputs in (segwit_inputs, taproot_inputs, mixed_inputs):
+    assert_no_change_warning(inputs, [valid_mixed_segwit_change])
+
+valid_mixed_taproot_change = MockOutput(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
+for inputs in (segwit_inputs, taproot_inputs, mixed_inputs):
+    assert_no_change_warning(inputs, [valid_mixed_taproot_change])
+
+wrong_tap_metadata_for_segwit = MockOutput(NATIVE_P2WPKH,
+                                           tap_subpaths={TAP_PUBKEY: (BIP86_CHANGE_SUBPATH, [])})
+validate_must_fail(wrong_tap_metadata_for_segwit,
+                   CHANGE_WRONG_ACCOUNT_TYPE)
+
+wrong_segwit_metadata_for_taproot = MockOutput(TAPROOT_SCRIPT, subpaths={PUBKEY: BIP84_CHANGE_SUBPATH})
+validate_must_fail(wrong_segwit_metadata_for_taproot,
+                   CHANGE_WRONG_ACCOUNT_TYPE)
+
+return_value.write(b'OK')

### ports/stm32/boards/Passport/modules/tests/unit/psbt_multisig_approval.py
@@ -9,6 +9,7 @@
 import stash
 import utils
 import uasyncio as asyncio
+from exceptions import CHANGE_ADDRESS_NOT_OURS
 from flows import Flow, SignPsbtCommonFlow
 from flows import sign_psbt_common_flow
 from public_constants import MUSIG_ASK, MUSIG_SKIP
@@ -88,8 +89,7 @@ def derive_path(self, path):
 
 class MockErrorPage:
     def __init__(self, text):
-        assert text == ('Transaction rejected. Passport could not verify that the change address '
-                        'belongs to this wallet.')
+        assert text == CHANGE_ADDRESS_NOT_OURS
 
     async def show(self):
         events.append('error')

### ports/stm32/boards/Passport/modules/wallets/utils.py
@@ -116,18 +116,25 @@ def get_addr_type_from_deriv(path):
     addr_type = get_addr_type_from_deriv_path(path)
     subpath = get_part_from_deriv_path(path, 4)
 
-    if addr_type == 44:
+    return get_addr_type_from_bip_numbers(addr_type, subpath)
+
+
+def get_addr_type_from_bip_numbers(purpose, script_type=None):
+    # Translate the BIP purpose (and, for BIP48, its script subtype) into the
+    # corresponding Passport address format. Keep this mapping shared by all
+    # callers that reason about BIP32 derivation metadata.
+    if purpose == 44:
         return AF_CLASSIC
-    elif addr_type == 49:
+    elif purpose == 49:
         return AF_P2WPKH_P2SH
-    elif addr_type == 84:
+    elif purpose == 84:
         return AF_P2WPKH
-    elif addr_type == 48:
-        if subpath == 1:
+    elif purpose == 48:
+        if script_type == 1:
             return AF_P2WSH_P2SH
-        elif subpath == 2:
+        elif script_type == 2:
             return AF_P2WSH
-    elif addr_type == 86:
+    elif purpose == 86:
         return AF_P2TR
 
     return None