Description

grantorVesting.releaseRate in transferVesting function is number of tokens that is going to be released on each steps. When calculating grantorVesting.releaseRate its only considered the current totalAmount and numOfSteps . But it should be considered grantorVesting.amountClaimed and grantorVesting.stepsClaimed in order to calculate releaseRate since releaseRate should be remaining balance / remaining claimable steps.

Proof of Concept

  function transferVesting(address _grantor, address _beneficiary, uint256 _amount) external {
        require(
            msg.sender == tokenIssuer || msg.sender == manager || msg.sender == vestingDeployer,
            "SS_StepVesting: unauthorized"
        );
        require(_beneficiary != address(0), "SS_StepVesting: beneficiary is zero");
        require(_amount > 0, "SS_StepVesting: amount is zero");
        Vesting storage grantorVesting = _vestings[_grantor];
        require(
            grantorVesting.totalAmount - grantorVesting.amountClaimed >= _amount,
            "SS_StepVesting: insufficient balance"
        ); // 3.8. Claimed amount not checked in transferVesting function

        grantorVesting.totalAmount -= _amount;
        grantorVesting.releaseRate = grantorVesting.totalAmount / numOfSteps;

        _createVesting(_beneficiary, _amount, grantorVesting.stepsClaimed, true);

        emit VestingTransferred(_grantor, _beneficiary, _amount);
    }

Here grantorVesting.releaseRate = grantorVesting.totalAmount / numOfSteps , but it should be correct as shown below ,

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

Consider this scenario ,

_vestings[bob].totalAmount = 100e18
_vestings[bob].amountClaimed = 0

numOfSteps = 10 and so releaseRate =  100e18/10 = 10e18

Assume bob claim his tokens for initial 3 steps , then his balance becomes

_vestings[bob].totalAmount = 100e18
_vestings[bob].amountClaimed = 30e18

bob will list(listVesting) 20e18 tokens. This amount can be listed since sellLimit is also 20e18 with default vestingSettings[plan].maxSellPercent which is 2000.

Due to this listing

_vestings[bob].totalAmount = 100e18 - 20e18 = 80e18
_vestings[bob].amountClaimed = 30e18

In this case (with current implementation of releaseRate )

grantorVesting.releaseRate = grantorVesting.totalAmount / numOfSteps
                           = 80e18 / 10
                           = 8e18  // still 7 steps can be collected with this vesting rate

But actual releaseRate should be

grantorVesting.releaseRate = (grantorVesting.totalAmount-grantorVesting.amountClaimed) / (numOfSteps - grantorVesting.stepsClaimed)
                           = (80e18 - 30e18)/(10-3) 
                           = 50e18/7
                           = 7.142e18 //// still 7 steps can be collected with this vesting rate

consider both scenarios bob unlist listing (unlistVesting) after claim period finished (consider here no one is buying that listing )

With current implementation of releaseRate bob is able to collect

claim amount = 30e18
unlist amount = 20e18
Vesting amount on 7 periods = 8e18 * 7
                            = 56e18
So total amount = 30e18 + 20e18 + 56e18 = 106e18 (This is above 100e18)

same calculation with correct releaseRate

claim amount = 30e18
unlist amount = 20e18
Vesting amount on 7 periods = 50e18/7 * 7
                            = 50e18
So total amount = 30e18 + 20e18 + 50e18 = 100e18 

Its clearly noticed that actual releaseRate and current implementation of releaseRate are two different which caused eventually user total claim is higher than actual amount he should claim. So it caused other users to claim less token amount.

impact

Due to incorrect implementation of releaseRate is caused users claim amount can be different as that they should be.

Recommended mitigation steps

Use this equation for releaseRate

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