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 vesting contract in tap-token has a feature to release a certain portion of amount directly called initialUnlock.

Here is how it is calculated when querying how much is vested at a point in time, Vesting::_vested:

File: tap-token/contracts/tokens/Vesting.sol

273:        _start = _start - __initialUnlockTimeOffset; // Offset initial unlock so it's claimable immediately
274:        return (_totalAmount * (block.timestamp - _start)) / _duration; // Partially vested

Unfortunately, there's an issue. The contract simply offsets the start time with the initial __initialUnlockTimeOffset (initialUnlock). Hence at the end of the vesting period, block.timestamp - start can become larger than _duration, vesting a larger amount than intended.

This will reset as soon as the _duration is reached so it requires a user to claim just before the _duration has reached its end.

Impact

Vestors in pools with initialUnlocks will be able to claim more vested than intended. This will impact vestors who claim later as there might not be enough tokens in the contract to cover their withdrawals.

Proof of Concept

Test in tap-token/test/Vesting.t.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.22;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {ERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

import {Vesting} from "tap-token/tokens/Vesting.sol";

contract MockToken is ERC20 {
    constructor() ERC20("Mock","Token") {}

    function mint(address to, uint256 amount) public {
        _mint(to,amount);
    }
}

contract VestingTest is Test {

    MockToken token = new MockToken();

    Vesting vesting;

    address ada = makeAddr("ada");

    function setUp() public {
        vesting = new Vesting({
            _cliff: 0,
            _duration: 100,
            _owner: address(this)
        });

        vm.warp(1000);
    }

    function testInitalUnlock() public {
        vesting.registerUser(ada, 100);

        token.mint(address(vesting),100);
        vesting.init({
            _token: IERC20(address(token)),
            _seededAmount: 100,
            _initialUnlock: 1_000
        });

        // one second before finish
        // ada has received more than intended amount
        vm.warp(1099);
        assertEq(vesting.vested(ada),109);
    }
}

Tools Used

Manual audit

Recommended Mitigation Steps

Consider rethinking the method of providing the initialUnlock, if you either want a less steep slope or a non-vesting period until it should start to vest again.