fix(solana): properly sanitize account counts
What the developer wrote
Authored by Jakub Janků
fix(solana): properly sanitize account counts
The parsed address counts were previously checked only using asserts
which are stripped out in release builds. As a result, the device could
misclassify the accounts in a TX and present a writable signer as
read-only. However, no funds were at risk, since a TX with invalid counts
is rejected by the network.
Moreover, one asserted bound was incorrect, at least one writable signer
must be present in each TX (the fee payer).
This commit replaces the asserts with proper if-raise checks that now
mirror the on-chain checks, see:
https://github.com/anza-xyz/solana-sdk/blob/06b8acf9dfc505da943e25274454a95a5bdaa42a/message/src/legacy.rs#L166-L178
https://github.com/anza-xyz/solana-sdk/blob/06b8acf9dfc505da943e25274454a95a5bdaa42a/message/src/versions/v0/mod.rs#L126-L138
[no changelog]
What changed, and why it matters
This commit fixes a bug in how Trezor's Solana app counted account roles inside a transaction. In release builds, safety checks were removed, so a malformed transaction could trick the device into showing a signer account as read-only on screen. The device would not actually sign anything harmful, because the Solana network would reject the bad transaction anyway. The fix replaces the removed checks with proper error handling and corrects one rule: every transaction must have at least one writable signer to pay fees.
No immediate user action is required because the network rejects malformed transactions. Users should ensure their firmware is updated to a version containing this commit to prevent any confusion from incorrect on-screen account labels.
Security signals we found
assert replaced with explicit runtime validation
incorrect bound on writable signer count corrected
validation aligned with upstream Solana SDK message checks
potential UI misclassification of signer as read-only in release builds
Evidence from the diff
The Solana transaction parser in Trezor firmware previously used Python assert statements to validate two account-count invariants: (1) the number of read-only signers must be less than the total required signers, and (2) the total number of addresses must be at least required_signers_count + num_read_only_addresses. Because assert is stripped in optimized/release builds, these checks disappeared in production firmware. The first assert’s bound was also wrong; it allowed num_signature_read_only_addresses == required_signers_count, which would leave zero writable signers, violating Solana’s requirement that the fee payer be a writable signer. The patch replaces the asserts with explicit if ... raise DataError checks that match Solana SDK validation logic. The on-screen account classification could therefore be misled in release builds, but no funds were at risk because the network rejects such transactions.
Changed components
core/src/apps/solana/transaction/__init__.pyTrezor Solana transaction parseraccount header parsing and address classificationInspect captured patch +6 / −5
### core/src/apps/solana/transaction/__init__.py
@@ -89,12 +89,13 @@ def _parse_header(self, serialized_tx_reader: BufferReader) -> None:
def _parse_addresses(self, serialized_tx_reader: BufferReader) -> None:
num_of_addresses = parse_var_int(serialized_tx_reader)
- # Read-only signers are always just a subset of all signers.
- assert self.num_signature_read_only_addresses <= self.required_signers_count
- assert (
+ if self.num_signature_read_only_addresses >= self.required_signers_count:
+ raise DataError("At least one writable signer required")
+ if (
num_of_addresses
- >= self.required_signers_count + self.num_read_only_addresses
- )
+ < self.required_signers_count + self.num_read_only_addresses
+ ):
+ raise DataError("Not enough addresses")
addresses: list[Address] = []
for i in range(num_of_addresses):Why this scored 40/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.