Attackers that have compromised signer keys can financially overwhelm the protocol to take it over

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-10-kleidi/blob/main/src/Timelock.sol#L521
https://github.com/code-423n4/2024-10-kleidi/blob/main/src/Timelock.sol#L550

Vulnerability details

Impact

There is no limit on the number of scheduled tranasctions in the Timelock. As a result, in some cases it may be viable for an attacker that has compromised the signer keys to financially overwhelm the protocol team. They would do that by submitting more transactions than the protocol has cash to cancel(in terms of gas fees).

Proof of Concept

Suppose the following scenario:

• A Kleidi safe instance is used to manage an admin role for a lending protocol that has 5 million in locked collateral. However, the protocol team only has 250k operating cashflow.
• A malicios entity manages to compromise the private keys of the Safe and schedules a massive number of transactions. Paying say 2 million in gas costs.
• The guardian pauses the protocol and the protocol team manages to gain control of the protocol via a recovery spell, but does not have enough available cash to actually cancel all malicios transactions.
• The attackers execute a malicios transaction and profit 3 million from draining the managed lending protocol.

Note that the guardian may attempt to pause the protocol before the attackers have managed to deploy the huge number of transactions. In the current version of the protocol, the guardian will be dossed because the function to pause will be running out of gas in the loop while canceling scheduled transactions, but this attack is viable regardless. It really depends on the cash that the protocol team has available, it is very easy to spend a huge amount on gas, or in this case make someone else spend a huge amount on gas, especially on mainnet. Right now at ~14 GWEI gas price, each block costs ~1200 USD. At 5 blocks per minute this is 360,000 USD per hour.

Recommended Mitigation Steps

Allow the protocol to set a variable for a maximum number of scheduled transaction at the same time. That can be customizeable via a cold signer transaction. For example, if the protocol does not expect more than 10 tranasctions at the same time in the worst case, they should be able to set that and avoid the above attack. The check should be enforced on the schedule and scheduleBatch functions.

Assessed type

Other

[ElliotFriedman]

Disagree with severity. Guardian can cancel all in flight transactions and stop new transactions from being queued. Then a recovery spell can be deployed to rotate the compromised cold signer keys.

Seems like a low issue to me.

The desired end user is an individual and not a DAO or protocol team.

[GalloDaSballo]

I'm not convinced that the protocol team would consider cancelling the malicious transactions
They would instead have to initiate recovery to replace the compromised signer

[ElliotFriedman]

If your cold signers were compromised and you had a guardian and social recovery spell set, you would pause the contract, which would cancel all malicious transactions, and the execute the recovery spell which would kick the compromised signers off of the multisig.

[GalloDaSballo]

I believe that this finding is a different take on another finding, which to seems valid, which is that if you send enough malicious TXs, pause stops working

I will most likely dup this under the same issue as I believe that spamming txs is only possible from Pause not working, meaning that Pause being breakable is the bug and this report is a consequence of that

[c4-judge]

GalloDaSballo marked the issue as duplicate of #4

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[ElliotFriedman]

Mitigated here: solidity-labs-io/kleidi#53

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[gesha17]

Hello @GalloDaSballo,

Thanks for judging. I think this shoudn't be considered the same attack as the DoS vector. They have different prerequisites/root causes, different target victims and very different severity(in my opinion). Please excuse me for the long post, I want to clearly outline the differences between the two attacks.

First, lets analyse when a DoS vector attack will actually be successful. There are two cases:

1. The user has set the timelock duration to be less than the recovery spell duration - it is a prerequisite for this attack to work. This will make the attack successful because the malicios transactions will become executeable the moment the signer keys are recovered and the timelock is unpaused. However, that would be a clear user misconfiguration, as it is clearly indicated in the documentation that this should not be configured this way, so it is very unlikely to happen. Also, if the user can get external professional help, he could post several back-to-back transactions to cancel the malicios ones the moment the timelock is unpaused. This is because both cancel and execute have the whenNotPaused modifier.

2. If the users have configured the above correctly, then the attacker basically has left to hope that the user doesnt notice the attack before the timelock time left to execute the malicios transctions becomes less than the recovery spell duration. In reality, if an attacker goes through the trouble of compromising signer keys, I think it is extremely unlikely they will resort to "hoping" and would just use this attack and flood the user with more transactions than he can cancel. Also, the external professional help point from 1. applies here as well.

Considering the above two points, I think in reality its extremely unlikely that the DoS attack will ever be used, so it can be considered a low severity issue.

In contrast, the attack described in this submission would be valid for any wallet, no matter its configuration. The attacker can basically guarantee himself a win. Even in the unlikely case that the user manages to cancel all malicios transactions, the cost for recovering his wallet will be easily in the tens of thousands of dollars, and possibly in the hundreds of thousands.

Finally, considering the arguments made above, I believe this submission and #24 provide vastly more value to the sponsor than just the DoS vector. In my humble opinion, it would be unfair to group this and the DoS vector under the same issue as that would negate any meaningful payout me or the warden from #24 would get for identifying the only issue that would lead to a guaranteed loss of funds in this contest. None of the other submissions identify it.

[DemoreXTess]

As I stated in #24 (comment) , the defender doesn't need to race against attacker. There is no guaranteed win condition for attacker. I don't agree with the escalation comment.