Minato7namikazi - important logic bug in the `split` function

[sherlock-admin4]

Minato7namikazi

High

important logic bug in the split function

Vulnerability Detail

function split(uint _tokenId, uint amount) external {
    // ... (other checks)

    // remove old data
    _remove_from(_tokenId, amount, unlock_time, _locked);

    // mint 
    ++tokenId;
    uint _newTokenId = tokenId;
    _mint(_to, _newTokenId);
    _deposit_for(_newTokenId, amount, unlock_time, locked[_newTokenId], DepositType.SPLIT_TYPE);
}

The bug is in the amount being removed from the original token and deposited into the new token. The function removes amount from the original token but then deposits the same amount into the new token. This means that the total locked amount remains the same, which is not the intended behavior of a split operation.

To fix this, we should:

1. Remove amount from the original token.
2. Deposit amount into the new token.
3. Update the original token's locked amount to be value - amount.

Recommendation

function split(uint _tokenId, uint amount) external {
    // ... (other checks)

    // remove amount from old token
    _remove_from(_tokenId, amount, unlock_time, _locked);

    // update the original token's locked amount
    locked[_tokenId].amount = int128(int256(value - amount));

    // mint new token
    ++tokenId;
    uint _newTokenId = tokenId;
    _mint(_to, _newTokenId);

    // deposit amount into new token
    _deposit_for(_newTokenId, amount, unlock_time, LockedBalance(0, 0), DepositType.SPLIT_TYPE);
}

line

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/VotingEscrow.sol#L1217

Tool used

Manual Review

[nevillehuang]

Invalid, amount is removed from original NFT and deposit for new NFT to be minted, no issues here