fix(core/monero): validate change address on sweep transactions
What the developer wrote
Authored by Petr Susil
fix(core/monero): validate change address on sweep transactions
The sweep shape (declared change amount 0, two outputs) is exempt from
the change ownership check, because on a sweep the change address is a
random throwaway address, not ours. But the shape is host-controlled, so
a malicious host can point change_dts.addr at the paying recipient and
have the exemption accept it. That output is then keyed as change in
step 6, derived from our own view key (a*R), leaving it unspendable by
both the recipient and the owner.
Reject the aliasing: an address we decline to validate must never be the
address of an output that carries money. Honest sweeps are unaffected --
their fake output carries amount 0.
Fixes: https://github.com/trezor/trezor-firmware/issues/7544
(cherry picked from commit ea6caf1e1875df05e983629df90d3dd217af4ab9)
What changed, and why it matters
This update fixes a bug in how Trezor handles certain Monero 'sweep' transactions. In a sweep, one output is a fake zero-value decoy with a random address, and the device normally skips checking that the change address belongs to the wallet. A malicious computer app could exploit that skip by claiming the real payment recipient's address is the change address. The device would then encrypt the recipient's output using the wallet's own secret view key, making the money effectively unspendable by the recipient and unrecoverable by the sender. The fix adds a check: if the unvalidated change address appears on any output that actually carries money, the transaction is rejected. Honest sweeps, where the decoy carries zero coins, still work normally.
Apply this patch and ensure firmware releases include it. Users who signed Monero sweep transactions with older firmware should verify that recipients could spend outputs; if not, recovery may require the sender's private view key. Wallet software interacting with Trezor should be updated to enforce the new behavior.
Security signals we found
Missing validation of host-controlled change address in sweep exemption path
Potential keying of a recipient output with the wallet's own view key (a*R), rendering funds unspendable
Subaddress recipient additionally loses required ADDITIONAL_PUBKEYS if misclassified as change
Fix adds an address-equality guard before accepting the sweep exemption
Comprehensive unit tests added for the vulnerable branch
Evidence from the diff
In core/src/apps/monero/signing/step_01_init_transaction.py, the _check_change function exempts sweep-shaped transactions (change amount 0, exactly two outputs) from proving ownership of the change address, because the change_dts.addr is expected to be a random one-time address for the zero-amount fake output. Since the sweep shape is host-controlled, a malicious host could set change_dts.addr to the real recipient’s address. Later, in step 6, that output would be classified as change and its one-time destination public key would be derived as aR using the wallet’s view key, instead of the recipient’s expected Hs(rA)G+B. The patch rejects the transaction if any output with a non-zero amount has an address equal to the unvalidated change address. Tests cover the aliasing attack against both standard and subaddress recipients, as well as negative cases ensuring normal sweeps and normal change flows still pass.
Changed components
Trezor Model T / Core Monero signing modulecore/src/apps/monero/signing/step_01_init_transaction.pyMonero sweep transaction handlingInspect captured patch +152 / −0
### core/.changelog.d/7544.fixed
@@ -0,0 +1 @@
+Monero: fix change address validation being skipped on sweep transactions.
### core/src/apps/monero/signing/step_01_init_transaction.py
@@ -275,6 +275,13 @@ def _check_change(
# that spends exactly 0 coins to a random address.
# See https://github.com/monero-project/monero/pull/1415
if change_index is None and state.output_change.amount == 0 and len(outputs) == 2:
+ # The change address is not validated as ours on this path -- for a sweep it is
+ # the random address of the fake output. It must therefore never be the address
+ # of an output that actually carries money, otherwise that output would be keyed
+ # as change (a*R) in step 6 and nobody would be able to spend it.
+ for out in outputs:
+ if out.amount and addr_eq(out.addr, change_addr):
+ raise signing.ChangeAddressError("Change address spends to a recipient")
state.mem_trace("Sweep tsx" if __debug__ else None)
return
### core/tests/test_apps.monero.change.py
@@ -0,0 +1,144 @@
+# flake8: noqa: F403,F405
+from common import * # isort:skip
+
+if not utils.BITCOIN_ONLY:
+ from trezor.enums import MoneroNetworkType
+ from trezor.messages import (
+ MoneroAccountPublicAddress,
+ MoneroTransactionDestinationEntry,
+ )
+
+ from apps.monero.signing import ChangeAddressError
+ from apps.monero.signing.state import State
+ from apps.monero.signing.step_01_init_transaction import (
+ _check_change,
+ _get_primary_change_address,
+ )
+ from apps.monero.xmr import crypto, crypto_helpers
+ from apps.monero.xmr.credentials import AccountCreds
+
+
+@unittest.skipUnless(not utils.BITCOIN_ONLY, "altcoin")
+class TestMoneroCheckChange(unittest.TestCase):
+ """
+ `_check_change` is the only thing standing between a malicious host and an
+ output keyed as change (a*R) in step 6. Cover every branch, including the
+ sweep exemption, where the change address is deliberately *not* ours.
+ """
+
+ def setUp(self):
+ self.state = State()
+ self.state.creds = AccountCreds.new_wallet(
+ crypto_helpers.decodeint(
+ bytes.fromhex(
+ "4ce88c168e0f5f8d6524f712d5f8d7d83233b1e7a2a60b5aba5206cc0ea2bc08"
+ )
+ ),
+ crypto_helpers.decodeint(
+ bytes.fromhex(
+ "f2644a3dd97d43e87887e74d1691d52baa0614206ad1b0c239ff4aa3b501750a"
+ )
+ ),
+ network_type=MoneroNetworkType.TESTNET,
+ )
+ self.state.account_idx = 0
+ # our own primary change address for account 0
+ self.ours = _get_primary_change_address(self.state)
+ # two unrelated addresses: a payment recipient and wallet2's throwaway
+ # address for the 0-amount fake output of a sweep
+ self.recipient = self._foreign_addr()
+ self.dummy = self._foreign_addr()
+
+ def _foreign_addr(self):
+ spend = crypto.scalarmult_base_into(None, crypto.random_scalar())
+ view = crypto.scalarmult_base_into(None, crypto.random_scalar())
+ return MoneroAccountPublicAddress(
+ spend_public_key=crypto_helpers.encodepoint(spend),
+ view_public_key=crypto_helpers.encodepoint(view),
+ )
+
+ def _dst(self, amount, addr, is_subaddress=False):
+ return MoneroTransactionDestinationEntry(
+ amount=amount, addr=addr, is_subaddress=is_subaddress
+ )
+
+ def _check(self, change_dts, outputs):
+ self.state.output_change = change_dts
+ _check_change(self.state, outputs)
+
+ # --- sweep shape ---------------------------------------------------------
+
+ def test_sweep_is_accepted(self):
+ # What wallet2 actually emits: the change entry *is* the 0-amount fake
+ # output, sent to a random address that is not ours.
+ change = self._dst(0, self.dummy)
+ outputs = [self._dst(1000, self.recipient), self._dst(0, self.dummy)]
+ self._check(change, outputs)
+
+ def test_sweep_change_aliasing_recipient_is_rejected(self):
+ # Malicious host points the unvalidated change address at the paying
+ # output, so step 6 would key it with our own view key (a*R).
+ change = self._dst(0, self.recipient)
+ outputs = [self._dst(1000, self.recipient), self._dst(0, self.dummy)]
+ with self.assertRaises(ChangeAddressError):
+ self._check(change, outputs)
+
+ def test_sweep_change_aliasing_subaddress_recipient_is_rejected(self):
+ # Same poison against a subaddress recipient. Besides the a*R branch,
+ # this would also hide the recipient from classify_subaddresses and
+ # suppress the ADDITIONAL_PUBKEYS the recipient needs to scan.
+ change = self._dst(0, self.recipient)
+ outputs = [
+ self._dst(1000, self.recipient, is_subaddress=True),
+ self._dst(0, self.dummy),
+ ]
+ with self.assertRaises(ChangeAddressError):
+ self._check(change, outputs)
+
+ def test_sweep_change_aliasing_the_fake_output_is_accepted(self):
+ # The fake output carries no money, so keying it as change is harmless.
+ change = self._dst(0, self.dummy)
+ outputs = [self._dst(1000, self.recipient), self._dst(0, self.dummy)]
+ self._check(change, outputs)
+
+ def test_sweep_shape_with_our_own_change_is_accepted(self):
+ change = self._dst(0, self.ours)
+ outputs = [self._dst(1000, self.recipient), self._dst(0, self.ours)]
+ self._check(change, outputs)
+
+ # --- non-sweep shapes ----------------------------------------------------
+
+ def test_no_change(self):
+ self._check(None, [self._dst(1000, self.recipient), self._dst(0, self.dummy)])
+
+ def test_change_to_ourselves(self):
+ change = self._dst(500, self.ours)
+ outputs = [self._dst(1000, self.recipient), self._dst(500, self.ours)]
+ self._check(change, outputs)
+
+ def test_foreign_change_is_rejected(self):
+ change = self._dst(500, self.recipient)
+ outputs = [self._dst(1000, self.recipient), self._dst(500, self.recipient)]
+ with self.assertRaises(ChangeAddressError):
+ self._check(change, outputs)
+
+ def test_change_not_among_outputs_is_rejected(self):
+ change = self._dst(500, self.ours)
+ outputs = [self._dst(1000, self.recipient), self._dst(500, self.dummy)]
+ with self.assertRaises(ChangeAddressError):
+ self._check(change, outputs)
+
+ def test_three_outputs_with_foreign_change_is_rejected(self):
+ # The sweep exemption must not extend past two outputs.
+ change = self._dst(0, self.recipient)
+ outputs = [
+ self._dst(1000, self.recipient),
+ self._dst(0, self.dummy),
+ self._dst(0, self.dummy),
+ ]
+ with self.assertRaises(ChangeAddressError):
+ self._check(change, outputs)
+
+
+if __name__ == "__main__":
+ unittest.main()Why this scored 74/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.