An veNFT owner/approved address can override the expiry date by calling increase_amount()

Summary

VotingEscrow :: increase_amount() contains a bug that can will cause the unlock time to be extended without the owner's permission.

Vulnerability Detail

increase-amount() function first calls _isApprovedOrOwner() to ensure that the caller is authorised to make this call.

function increase_amount(uint _tokenId, uint _value) external nonreentrant {
        assert(_isApprovedOrOwner(msg.sender, _tokenId));

        LockedBalance memory _locked = locked[_tokenId];

        assert(_value > 0); // dev: need non-zero value
        require(_locked.amount > 0, 'No existing lock found');
        require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');

        _deposit_for(_tokenId, _value, 0, _locked, DepositType.INCREASE_LOCK_AMOUNT);
    }

_isApprovedOrOwner() calls max_lock which is the cause of the problem.

 function max_lock(uint _tokenId) public {
        if(maxLockIdToIndex[_tokenId] !=0 && max_lock_enabled) {
            LockedBalance memory _locked = locked[_tokenId];
            uint unlock_time = (block.timestamp + MAXTIME) / WEEK * WEEK; // Locktime is rounded down to weeks

            if(unlock_time > _locked.end) {
                require(_locked.end > block.timestamp, 'Lock expired');
                require(_locked.amount > 0, 'Nothing is locked');
                require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 52 weeks max');

                _deposit_for(_tokenId, 0, unlock_time, _locked, DepositType.INCREASE_UNLOCK_TIME);
            }
        }
    }

calling max_lock will extend the unlock_time of the token -on contrary to what is stated in the Natspec of the function - and that may be undesired action to the token holder.

Impact

Making changes to the token parameters without its owner's permission or even knowledge will jeopardize the protocol's efficiency and user experience.

Code Snippet

Tool used

Manual Review

Recommendation

Omit max_lock() from _isApprovedOrOwner()

function _isApprovedOrOwner(address _spender, uint _tokenId) internal returns (bool) {
     -   max_lock(_tokenId);
         ...
    }

Duplicate of #622

sherlock-audit / 2024-06-velocimeter-judging

FlyingBird - An veNFT owner/approved address can override the expiry date by calling increase_amount() #618