Handle

Dravee

Vulnerability details

Impact

To give more trust to users: functions that set key/critical variables should be put behind a timelock.

While it's good that there's a MIN_BALANCE_SANITY_CEILING constant that prevents setting a value that is too high to be reasonable, a change in setMinActiveBalance() might put some protocols in a position where they become removable and their funds gets transfered to an agent, to be later claimed.

These protocols might want to deposit a little more in advance to stay in the game instead of being forced to go through the claim & re-deposit process. Or they might not trust the process and want to withdraw everything by themselves.

Proof of Concept

setMinActiveBalance() that sets the condition in forceRemoveByActiveBalance() : https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/SherlockProtocolManager.sol#L419-L427

forceRemoveByActiveBalance() that should also be put behind a timelock: https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/SherlockProtocolManager.sol#L624-L629

Tools Used

VS Code

Recommended Mitigation Steps

Add a timelock to setter functions of key/critical variables. I saw a comment in protocolRemove() saying that the call should be subject to a timelock, but I didn't see the same comment on these other 2 functions

code-423n4 / 2022-01-sherlock-findings