search

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

0 stars 0 forks source link

User facing and asset bearing Vault and BaseStrategy contracts miss emergency lever #154

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

hyh

Vulnerability details

Impact

If there be any emergency with system contracts or outside infrastructure, there is no way to temporary stop the operations.

Proof of Concept

Vault: https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/Vault.sol

BaseStrategy: https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/strategy/BaseStrategy.sol

Recommended Mitigation Steps

Make user facing contracts, first of all Vault and Strategies, pausable. For example, by using OpenZeppelin's approach: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol

It will be prudent to perform a deeper check to draw a list of endpoints that require to be paused if something unexpected happens.

naps62 commented 2 years ago

It is a valid suggestion, but does not hint at any actual risk in the contracts, just speculates about it.

If an emergency event happens, there is already the capability of withdrawing all funds from the strategy, and eventually switch to a new one. This already mitigates a lot of the risk (would theoretically turn the vault into a deposit/withdraw vault with no yield whatsoever)

dmvt commented 2 years ago

As the sponsor mentions, they could pull funds from the strategies, temporarily halting operations. This issue is invalid.