Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Immutable Recovery threshold limits flexibility, risking loss of Safe access in critical scenarios #27

Closed
howlbot-integration bot opened this issue Oct 27, 2024 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-16 nullified Issue is high quality, but not accepted 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-10-kleidi/blob/ab89bcb443249e1524496b694ddb19e298dca799/src/RecoverySpell.sol#L51
https://github.com/code-423n4/2024-10-kleidi/blob/ab89bcb443249e1524496b694ddb19e298dca799/src/RecoverySpell.sol#L56

Vulnerability details

Summary

The RecoverySpell contract is a critical component of the Kleidi system, intended to enable secure recovery of Safe accounts in the event that an owner loses access to their private key. However, the contract’s design makes the recovery threshold immutable, meaning it cannot be adjusted once set. This inflexibility poses a risk: if one of the recovery participants also loses their access key, it becomes impossible to meet the threshold required for recovery. This design flaw undermines one of the primary functions of the Kleidi system, potentially locking users out of their funds.

Proof of Concept

In RecoverySpell, the threshold and recoveryThreshold values are defined as immutable, meaning they are assigned at deployment and cannot be modified later. If an owner (Joe) creates a RecoverySpell module with a threshold based on his wife, son, and friend as authorized participants, but later, one of these participants also loses their key, recovery becomes impossible due to the fixed threshold. This inability to adapt in emergencies prevents Joe from recovering his Safe, ultimately blocking access to his funds forever.

Impact

The inability to adjust recovery thresholds results in a critical loss of flexibility in emergency scenarios. If a recovery participant loses their key, the immutability of the threshold blocks the entire recovery process. This can effectively render the Safe unrecoverable, exposing users to potential loss of funds, particularly in multi-participant recovery setups.

Recommended Mitigation Steps

Introduce a mechanism to adjust the recovery threshold dynamically if needed in emergency cases. One approach would be to allow a majority of participants (e.g., recoveryAddressArr.length - 1) to approve a reset of the threshold under special conditions. This would maintain security while providing critical flexibility for recovery in cases where one or more participants are unavailable.

Assessed type

Context

@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_primary AI based primary recommendation bug Something isn't working duplicate-16 sufficient quality report This report is of sufficient quality labels Oct 27, 2024
howlbot-integration bot added a commit that referenced this issue Oct 27, 2024
@c4-judge c4-judge added the nullified Issue is high quality, but not accepted label Nov 4, 2024
@c4-judge
Copy link

c4-judge commented Nov 4, 2024

GalloDaSballo marked the issue as nullified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-16 nullified Issue is high quality, but not accepted 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant