Wrench attack resistance does not provide the advertised security guarantees

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-10-kleidi/blob/ab89bcb443249e1524496b694ddb19e298dca799/src/Timelock.sol#L687-L700
https://github.com/code-423n4/2024-10-kleidi/blob/ab89bcb443249e1524496b694ddb19e298dca799/src/Timelock.sol#L815-L820

Vulnerability details

Vulnerability Details

Impact

The documentation states that when the cold signers of a safe are compromised, the pause guardian can pause the contract to cancel all malicious proposals, preventing them from being executed. After that, the recovery spell can be triggered to rotate the signing keys.

However, when the pause guardian calls pause(), a new pause guardian must be set by calling setGuardian().
This can only be done by the Timelock, meaning the proposal to set a new guardian has to wait x days until it can be executed.
The waiting period is determined by the delay (hereafter referred to as timelockDelay) of the Timelock, which has a maximum of 30 days.
Furthermore a proposal can only be scheduled after the pauseDuration has ended.

As a result, the contract may remain unprotected for at least timelockDelay if the pause guardian needs to pause the contract.

If the cold signers are compromised again during this time, there will be no way to stop them from executing malicious proposals, which will cause a user to lose his funds.

Proof of Concept

The following values are chosen for the important variables:

timelockDelay = 5 days
delay of RecoverySpellA = 10 days
delay of RecoverySpellB = 10 days
pauseDuration = 11 days

It is important to note that the delay of the recovery spells does not have the same purpose as the timelockDelay.

1. The cold signers are compromised.

2. RecoverySpellA is created to rotate the signing keys.

3. The pause guardian calls pause() to cancel all proposals, including the malicious ones, and gets deleted.

4. After the 10 days, executeRecoverySpell() gets called to rotate the signing keys.

5. The Safe schedules a new proposal to set a new guardian, which can be executed after the 5 days.

6. The cold signers of the safe get compromised again before the timelockDelay is over and call cancel() to cancel the proposal to set a new guardian. They also schedule malicious proposals.

7. RecoverySpellB is created to rotate the signing keys.

8. As there is no guardian to pause the contract, the malicious proposals will be executed before the delay of RecoverySpellB runs out.

Recommended Mitigation Steps

Consider allowing a user to set multiple guardians.
It is important to make sure that this is compatible with the whole protocol.
Furthermore, it is insufficient to provide security measures that only work under the assumption of one compromise attempt.
While these are sufficient to protect against one $5 wrench attack, they simply raise the cost to a two times $5 wrench attack.
In particular it is not necessary to kidnap a user for 30 days. It is sufficient to hit him with a $5 wrench on two different days.

Assessed type

MEV

[HollaDieWaldfee100]

This finding didn't make it into the findings repo.
Please see the high level thoughts in this separate comment (code-423n4/2024-10-kleidi-findings#14 (comment)). I believe these high level thoughts apply to both findings.

With regards to wrench attack resistance, the documentation on the contest page (https://code4rena.com/audits/2024-10-kleidi) states that:

Wrench Resistant: One of the guiding principles of this system is to be resistant to $5 wrench attacks. Even if an attacker is able to coerce a user into signing a transaction, the system of recovery spells and guardians slows down an attacker trying to steal funds. With a 30 day timelock as the delay on new transactions, an attacker would need to kidnap a user for a month and remain undetected for the entire period in order to steal funds.

In particular the last sentence:

an attacker would need to kidnap a user for a month and remain undetected for the entire period in order to steal funds.

We show that, while using an intended system configuration, it is enough to use the wrench attack on two different days rather than to kidnap someone for the whole pause time and remain undetected
In other words, the problem is that one ongoing recovery attempt completely exposes the user to the next $5 wrench attack.

In a different scenario, an attacker could just scare a user into using a recovery spell by social engineering and then execute one $5 wrench attack.

I agree we're constructing a scenario here that is not straightforward and hence is not High severity.

But given the rules of Medium severity on the one hand:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

and that the main selling point is the $5 wrench attack resistance on the other hand, I believe Medium severity is appropriate.

[gesha17]

This finding should be a dupe with my submission 6 from the findings repo. I submitted it as a low but it looks like a valid medium severity concern, since the protocol will be unprotected for some duration.

[GalloDaSballo]

• The cold signers of the safe get compromised again before the timelockDelay is over and call cancel() to cancel the proposal to set a new guardian. They also schedule malicious proposals

When keys are compromised, multiple parties can execute transactions, since all transactions are behind a timelock, all transactions will be cancelled, making it so that no transaction will be executable
Because all parties will be able to cancel each other transactions since that has no delay

Meaning that Recovery Spell 2 will not be disrupted and instead will be able to work given a scenario of 2 consecutive attacks

[HollaDieWaldfee100]

When keys are compromised, multiple parties can execute transactions, since all transactions are behind a timelock, all transactions will be cancelled, making it so that no transaction will be executable
Because all parties will be able to cancel each other transactions since that has no delay

Okay, agree with you on this. This finding requires a key compromise in the sense of stealing a key such that the original owner of them loses access (for example I go to your house and take your seeds).

[gesha17]

When keys are compromised, multiple parties can execute transactions, since all transactions are behind a timelock, all transactions will be cancelled, making it so that no transaction will be executable Because all parties will be able to cancel each other transactions since that has no delay

This can hindered. The Safe implementation accepts a nonce as a part of the transaction hash which is then checked against the signature, meaning malicios actor can frontrun honest txs to increase the nonce and make them revert. Thus the honest user won't be able to cancel the malicios transactions.