FIAT DAO veFDT contest - yixxas's results

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L507-L515 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L355-L362

Vulnerability details

Impact

Global slope and global bias ( i.e voting power ) is not calculated correctly every time someone makes a call to increaseUnlockTime() when _checkpoint() VotingEscrow.sol#L514 is called and thus can affect reliability of the protocol.

Proof of Concept

oldLocked is a copy of locked_ on L512 and oldLocked.end = unlock_time on L513, but no change is done as L507 has already set the original locked_.end = unlock_time . This means that the parameters that are passed into _checkpoint() L514, oldLocked and locked_ has the exact same values.

VotingEscrow.sol#L507-L515

function increaseUnlockTime(uint256 _unlockTime){
        ...
        locked_.end = unlock_time;
        locked[msg.sender] = locked_;
        if (locked_.delegatee == msg.sender) {
            // Undelegated lock
            require(oldUnlockTime > block.timestamp, "Lock expired");
            LockedBalance memory oldLocked = _copyLock(locked_);
            oldLocked.end = unlock_time;
            _checkpoint(msg.sender, oldLocked, locked_);
        }
...
}

Every time a user calls increaseUnlockTime() to extend their lock, lastPoint.slope and lastPoint.bias does not change in _checkpoint() since oldLocked == newLocked.

VotingEscrow.sol#L355-L362

 function _checkpoint(address _addr,LockedBalance memory _oldLocked,LockedBalance memory _newLocked){
            ...
            lastPoint.slope =
                lastPoint.slope +
                userNewPoint.slope -
                userOldPoint.slope;
            lastPoint.bias =
                lastPoint.bias +
                userNewPoint.bias -
                userOldPoint.bias;
...
}

Recommended Mitigation Steps

Make copy of _locked before making the changes to locked_.end

function increaseUnlockTime(uint256 _unlockTime){
        ...
        LockedBalance memory oldLocked = _copyLock(locked_);
        locked_.end = unlock_time;
        locked[msg.sender] = locked_;
        if (locked_.delegatee == msg.sender) {
            // Undelegated lock
            require(oldUnlockTime > block.timestamp, "Lock expired");
            _checkpoint(msg.sender, oldLocked, locked_);
        }
...
}