GalloDaSballo
commented
2 years ago The warden has shown how using the Shelter can cause depositors to loose yield they had accrued.
Specifically the loss of yield will be for the time of new rewards accrued since the last _checkpoint
Moving to a global index (to compare user accrual against global rewards), or modifying the shelter to account for yield could be a potential way to mitigate.
The sponsor confirmed and I believe medium severity to be appropriate because this is a Owner Privilege + Yield Loss finding
Lines of code
https://github.com/code-423n4/2022-02-concur/blob/shelter-client/contracts/ConvexStakingWrapper.sol
Vulnerability details
Impact
The shelter mechanism aims to protect the protocol's users by draining funds into a separate contract in the event of an emergency. However, while users are able to reclaim their funds through the
Shelter.solcontract, they will still have a deposited balance from the perspective ofConvexStakingWrapper.sol.Because users will only receive their rewards upon depositing/withdrawing their funds due to how the checkpointing mechanism works, it is likely that by draining funds to the
Shelter.solcontract, users will lose out on any rewards they had accrued up and until that point. These rewards are unrecoverable and can potentially be locked within the contract if the reward token is unique and only belongs to the sheltered_pid.Proof of Concept
https://github.com/code-423n4/2022-02-concur/blob/shelter-client/contracts/ConvexStakingWrapper.sol
Tools Used
Manual code review.
Recommended Mitigation Steps
Consider allowing users to call a public facing
_checkpointfunction once their funds have been drained to theShelter.solcontract. This should ensure they receive their fair share of rewards. Careful consideration needs to be made when designing this mechanism, as by giving users full control of the_checkpointfunction may allow them to continue receiving rewards after they have withdrawn their LP tokens.