c4-bot-9 commented 4 months ago

Lines of code

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/tokens/Vesting.sol#L273-L274

Vulnerability details

Description

The variable __initialUnlockTimeOffset is introduced to facilitate the initial unlock process. It's computed within the Vesting.init() function and later utilized in the Vesting._vested() function to adjust the start time, allowing users to claim their vested amounts immediately.

function _vested(uint256 _totalAmount) internal view returns (uint256) {
    uint256 _cliff = cliff;
    uint256 _start = start;
    uint256 _duration = duration;

    if (_start == 0) return 0; // Vesting not initiated

    if (_cliff > 0) {
        _start = _start + _cliff; // Apply cliff offset
        if (block.timestamp < _start) return 0; // Cliff period not reached
    }

    if (block.timestamp >= _start + _duration) return _totalAmount; // Fully vested

    _start = _start - __initialUnlockTimeOffset; // Adjust initial unlock time for immediate claiming
    return (_totalAmount * (block.timestamp - _start)) / _duration; // Calculate partially vested amount
}

However, the calculation performed by the function above is accurate only when block.timestamp == _start. To illustrate this point, consider the following scenario:

The Vesting contract's state is as follows:
- cliff = 0
- start = 100
- duration = 100
- user[Alice].amount = 50
- The initial unlock _initialUnlock = 30. After the init() function is executed, the value of __initialUnlockTimeOffset is calculated as 100 - (100 - 30 * 100 / 100) = 30.

At block.timestamp = 190, Alice calls the Vesting.claim() function to claim her assigned amount. Subsequently, the vested(user[Alice].amount = 50) function is invoked to compute Alice's vested amount.

vested(_totalAmount = 50)
   line 273: 
        _start = _start - __initialUnlockTimeOffset = 100 - 30 = 70
   line 274: 
        return 
           (_totalAmount * (block.timestamp - _start)) / _duration
           =   (50 * (190 - 70)) / 100 
           =   60

As observed in the example above, Alice is able to claim 60 tokens, which exceeds her assigned amount by more than 10 tokens.

Impact

The attacker can claim more tokens than the amount assigned, leading to an insufficient amount of tokens available for other users to claim.

Tools Used

Manual review

Recommended Mitigation Steps

Consider eliminating the usage of the variable __initialUnlockTimeOffset and instead introducing a new variable called initialUnlockAmount. Implement a new function allowing users to claim this amount at any time.

function claimInitialUnlockAmount() external {
    uint userClaimedAmount = initialUnlockAmount * users[msg.sender].amount / totalRegisteredAmount; 
    token.safeTransfer(msg.sender, userClaimedAmount);
}