//
User Can Claim More Than Allocated Amount
ITCruiser profile imageITCruiser
High

Summary

The transferVesting() function in the SecondSwap_StepVesting.sol contract contains an incorrect calculation for the releaseRate. This issue makes the users to claim more than allocated amount.

Description

When a user calls the listVesting() function in SecondSwap_Marketplace.sol with _amount = sellAmount, the releaseRate is incorrectly calculated as (totalAmount - sellAmount) / numOfSteps . The correct calculation for releaseRate should be (totalAmount - amountClaimed - sellAmount) / (numOfSteps - stepsClaimed).

The user's claimable amount is calculated as totalAmount - amountClaimed - sellAmount, but they can actually claim the following amount:

       releaseRate * (numOfSteps - stepsClaimed)

This leads to the following calculation for deltaAmount:
deltaAmount = releaseRate * (numOfSteps - stepsClaimed) - (totalAmount - amountClaimed - sellAmount)
= ((totalAmount - sellAmount) / numOfSteps) * (numOfSteps - stepsClaimed) - (totalAmount - amountClaimed - sellAmount)
= -(totalAmount - sellAmount) * stepsClaimed / numOfSteps + amountClaimed
= -(totalAmount - sellAmount) * stepsClaimed / numOfSteps + stepsClaimed * totalAmount / numOfSteps
= stepsClaimed * sellAmount / numOfSteps

Thus, the user can gain an additional benefit equal to deltaAmount. In this case, the last step may cause an underflow due to the following code:
https://github.com/code-423n4/2024-12-secondswap/blob/214849c3517eb26b31fe194bceae65cb0f52d2c0/contracts/SecondSwap_StepVesting.sol#L178-L182

if (vesting.stepsClaimed + claimableSteps >= numOfSteps) {
    //[BUG FIX] user can buy more than they are allocated
    claimableAmount = vesting.totalAmount - vesting.amountClaimed;
    return (claimableAmount, claimableSteps);
}

Therefore, the actual benefit is calculated as follows:
benefit = deltaAmount - releaseRate * 1
= stepsClaimed * sellAmount / numOfSteps - (totalAmount - sellAmount) / numOfSteps
= (stepsClaimed * sellAmount - totalAmount + sellAmount) / numOfSteps

Root Cause

The transferVesting() function in the SecondSwap_StepVesting.sol contract contains an incorrect calculation for the releaseRate.

        grantorVesting.releaseRate = grantorVesting.totalAmount / numOfSteps;

Proof of Concept

  • Consider a vesting with the following parameters:

    • vesting.numOfSteps = 10
    • totalAmount = 100
    • stepsClaimed = 5
    • amountClaimed = 50
    • releaseRate = 10
  • When the user calls listVesting() function with _amount = 40 in SecondSwap_Marketplace.sol:

    • totalAmount = 60
    • stepsClaimed = 5
    • amountClaimed = 50
    • releaseRate = 60 / 10 = 6 // incorrect calculation
  • The user can claim 4 times:

    • claimableAmount = 6 * 4 = 24
    • deltaAmount = 24 - (60 - 50) = 14

User gained 14 tokens.

Mitigation

Correct the releaseRate updating code.
https://github.com/code-423n4/2024-12-secondswap/blob/214849c3517eb26b31fe194bceae65cb0f52d2c0/contracts/SecondSwap_StepVesting.sol#L230

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

Links to affected code