Fixed: device wallet front-running random eSIM identifier - #117
Merged
Merged
Conversation
In-line with main
The identifier was written by the owning device wallet, which let an owner set a string the registry had no record of. Assigning and claiming now happen together in one admin-only path, so the two slots cannot disagree. setESIMUniqueIdentifier is onlyRegistry, and the device wallet no longer carries the setter or the modifier that guarded it.
full/ verifies a whole spec, scoped/ narrows to a few rules where the full run is too slow or needs a different bound, probe/ is for one-off checks.
Notes on the vacuous records the logs carry: postCreateAccount is unreachable because the prover gives code only to contracts in the scene, not because of the hash bound, so the duplicate report guard is unproved and covered by the unit tests instead. The registry setter vacuity in ESIMWalletFactory is expected, since the rule needs a state the setter refuses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates:
certorafolderBug Fix: Move eSIM identifier assignment to the registry
What changed
Registry.claimESIMIdentifierremoved.Registry.assignESIMIdentifier(eSIMWallet, identifier)replaces it, gated ononlyESIMWalletAdmin.DeviceWallet.setESIMUniqueIdentifierForAnESIMWalletand itsonlyESIMWalletAdminOrRegistrymodifier deleted. Device wallets no longer touch identifiers.ESIMWallet.setESIMUniqueIdentifiermoved fromonlyDeviceWallettoonlyRegistry.RegistryHelper._assignESIMIdentifier. The lazy deployment path calls the same one.The attack this closes
claimESIMIdentifieraccepted a call from any registered device wallet. Its only check was that the eSIM wallet named in the call belonged to the caller.claimESIMIdentifier(victimIdentifier, attackerOwnedWallet)with more gas. The ownership check passes, because they really do own the wallet they named.claimedESIMIdentifiersis never cleared. The victim's assignment then reverts withESIMIdentifierAlreadyClaimedpermanently, and the eSIM they paid for cannot be delivered.Coverage
NemesisIdentifierSquatPoC.t.solreplays the original attack and shows it reverting on the first line.IdentifierCollision.t.soland the device wallet guard tests now driveregistry.assignESIMIdentifier.