fix(core): check group thresholds of super-shamir
What the developer wrote
Authored by obrusvit
fix(core): check group thresholds of super-shamir
- check the individual group thresholds before comitting the backup
flags
- this prevents a malformed param msg to get the device into the state
of "Backup failed"
What changed, and why it matters
This update fixes a validation gap in the Trezor hardware wallet's Advanced Shamir backup setup. Previously, the device could accept malformed backup settings (such as too many groups or invalid member counts) and end up stuck in a 'Backup failed' state. The fix adds earlier checks so bad settings are rejected before the device commits to the backup process.
Users should update to firmware containing this fix. When configuring Advanced Shamir backups, ensure group and member parameters are within documented limits. Developers should verify that all backup parameter validation occurs before any persistent state is written.
Security signals we found
Input validation added before state-changing operation
Prevents device from entering 'Backup failed' state due to malformed parameters
Refactors duplicated validation logic into a single helper
Adds explicit maximum group count enforcement
Evidence from the diff
The commit adds validation in backup_device.py before the backup flags are committed. It now enforces a maximum group count via slip39.MAX_GROUP_COUNT and validates each group’s member threshold and member count using a new check_member_parameters() helper in slip39.py. That helper reuses the existing _check_parameters() logic and also forbids 1-of-N member configurations. The previous validation only happened later inside split_ems(), after the device had already entered the backup state, which could leave it in an unfinished/failed backup condition.
Changed components
core/src/apps/management/backup_device.pycore/src/trezor/crypto/slip39.pyAdvanced Shamir backup featureInspect captured patch +24 / −7
### core/.changelog.d/7754.fixed
@@ -0,0 +1 @@
+Check the correctness of thresholds in Advanced Shamir backup to prevent unfinished backup state.
### core/src/apps/management/backup_device.py
@@ -89,6 +89,7 @@ async def perform_backup(
async def backup_device(msg: BackupDevice) -> Success:
from trezor import wire
+ from trezor.crypto import slip39
from trezor.messages import Success
from apps.common import backup, mnemonic
@@ -113,8 +114,19 @@ async def backup_device(msg: BackupDevice) -> Success:
raise wire.DataError("group_threshold must be a positive integer")
if len(groups) < group_threshold:
raise wire.DataError("Not enough groups provided for group_threshold")
+ if len(groups) > slip39.MAX_GROUP_COUNT:
+ raise wire.DataError(
+ f"Too many groups provided, max is {slip39.MAX_GROUP_COUNT}"
+ )
if mnemonic.is_bip39():
raise wire.ProcessError("Expected SLIP39 backup")
+
+ for n, (member_threshold, member_count) in enumerate(groups):
+ try:
+ slip39.check_member_parameters(member_threshold, member_count)
+ except ValueError:
+ raise wire.DataError(f"Invalid group {n}")
+
elif len(groups) > 0:
raise wire.DataError("group_threshold is missing")
### core/src/trezor/crypto/slip39.py
@@ -211,13 +211,8 @@ def split_ems(
f"The requested group threshold ({group_threshold}) must not exceed the number of groups ({len(groups)})."
)
- if any(
- member_threshold == 1 and member_count > 1
- for member_threshold, member_count in groups
- ):
- raise ValueError(
- "Creating multiple member shares with member threshold 1 is not allowed. Use 1-of-1 member sharing instead."
- )
+ for member_threshold, member_count in groups:
+ check_member_parameters(member_threshold, member_count)
# Split the Encrypted Master Secret on the group level.
group_shares = _split_secret(group_threshold, len(groups), encrypted_master_secret)
@@ -537,6 +532,15 @@ def _check_parameters(threshold: int, share_count: int) -> None:
)
+def check_member_parameters(member_threshold: int, member_count: int) -> None:
+ _check_parameters(member_threshold, member_count)
+
+ if member_threshold == 1 and member_count > 1:
+ raise ValueError(
+ "Creating multiple member shares with member threshold 1 is not allowed. Use 1-of-1 member sharing instead."
+ )
+
+
def _split_secret(
threshold: int, share_count: int, shared_secret: bytes
) -> list[tuple[int, bytes]]:Why this scored 44/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.