search

code-423n4 / 2022-08-fiatdao-findings

2 stars 1 forks source link

Expired locks can still `increaseUnlockTime` when the `locked_.end != msg.sender` #181

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L493-L523

Vulnerability details

Impact

Expired locks can still increase their unlock time and the new unlock_time does not factor in the duration the lock was inactive

Proof of Concept

Users can increase their unlock time via the increaseUnlockTime () function. In the increaseUnlockTime () function as shown below, there is no check to ensure that users only increase their unlock time when their lock has not yet expired as locked_.end > block.timestamp. This check only occurs when the locked_.delegatee == msg.sender

  1. Alice locks 1000 tokens in the contract for 1 week.
  2. Alice then delegates her locks to Bob
  3. After 3 weeks, Alice locks are expired
  4. Alice then on week 4 increases her locks and such her locked_.end += 5 weeks.
  5. The contract would still accumulate her locks end without reducing the 3 weeks the locks were expired.

    Tools Used

    Manual Review

    Recommended Mitigation Steps

    Prohibit users to increase their lock duration when the locks are already expired.

lacoop6tu commented 2 years ago

This is an expected behaviour, it ensures a user can undelegate or re-delegate (when delegating the delegator inherits the delegatee lock end)

gititGoro commented 2 years ago

This prevents bad states from arising. Marking invalid