veRWA - RandomUser's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L331 https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L384 https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L301

Vulnerability details

Impact

Function VotingEscrow::withdraw() in line 331 requires that sender should be his own delegatee, but when user alerady delegated his funds to someone he has to call function VotingEscrow::delegate() with user's address as argument in order to set variable "delegatee" to his address. Then line 384 in that function ensures that new deletatee's lock ends later than former's delegatee. User also can augment his lock time for 5 years in any minute thanks to VotingEscrow::increaseAmount() function. Thanks to that condition Malicous delegatee can block withdrawals of all user that decided to delegate to him by calling function Voting::increaseAmount() thereby increasing his LockedBalance::lock time and making it higher than users LockedBalance::lock time. This will prevent them from changing delegatee which is necessary to withdraw they CANTO

Proof of Concept

Consider the following scenario :

1 - User A creates a lock VotingEscrow::createLock() with huge amount of CANTO, 2 - Then delegates VotingEscrow::delegate() that amount to malicious user B, 3 - User B deliberately calls VotingEscrow::increaseAmount() near the end of the user's 1 lock in order to increase his end time to maximum,

Now user A can't change LockedBalance::delegatee to himself in order to withdraw his CANTO becasuse his LockedBalance::end is lower than user's B; So the only way of user A to withdraw his CANTO is to extend his lock and change delegatee; But because of that he loses his liqudity for another 5 years;

Tools Used

Manual Review

Recommended Mitigation Steps

Consider change line 384 in VoteEscrow to the following condition:

if (_addr != msg.sender) { require(toLocked.end >= fromLocked.end, "Only delegate to longer lock"); }

Assessed type

Invalid Validation