Finding description and impact

The token issuer has the ability to change vesting allocation. However, an user can prevent his vesting from being allocated by listing his vesting to the marketplace.

The function SecondSwap_StepVesting::transferVesting() can be used by token issuer to transfer vesting, effectively reallocate vestings. By listing the vesting to marketplace, seller's allocated amount is sent to VestingManager contract, which can make the token issuer unable to reallocate his vesting directly (due to available amount check). Indeed, if the token issuer decides to reallocate that wanted amount from VestingManager, then this can cause marketplace insolvent.

    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);
    }

    function listVesting(address seller, address plan, uint256 amount) external onlyMarketplace {
        require(vestingSettings[plan].sellable, "vesting not sellable");
        require(SecondSwap_Vesting(plan).available(seller) >= amount, "SS_VestingManager: insufficient availablility");

        Allocation storage userAllocation = allocations[seller][plan];

        uint256 sellLimit = userAllocation.bought;
        uint256 currentAlloc = SecondSwap_Vesting(plan).total(seller);

        if (currentAlloc + userAllocation.sold > userAllocation.bought) {
            sellLimit +=
                ((currentAlloc + userAllocation.sold - userAllocation.bought) * vestingSettings[plan].maxSellPercent) /
                BASE;
        }

        userAllocation.sold += amount;

        require(userAllocation.sold <= sellLimit, "SS_VestingManager: cannot list more than max sell percent");
@>        SecondSwap_Vesting(plan).transferVesting(seller, address(this), amount);
    }

Note that: This attack vector can be done by front-running, since the codebase is deployed to Ethereum

Impacts:

• Token issuer can not reallocate as expected. At least, the total allocated amount can not be reallocated, depending on the max sell percent

Proof of Concept

Add this test below to the file test/VestingManager.test.ts, under describe("List, Purchase and Transfer".

    describe("List, Purchase and Transfer", async function () {
    ...

        it.only("prevent allocation", async function () {
          // initial, alice has 1000
          const {vesting, alice, bob, manager, tokenOwner,owner, token, marketPlaceholder} = await loadFixture(deployStepVestingFixture);
          
          // token issuer wants to reallocate 1000 from alice
          // but alice front-runs to list to marketplace
          await manager.write.listVesting([alice.account.address, vesting.address, parseEther("200")], { account: marketPlaceholder.account });
          
          // can not reallocate 1000
          await expect(vesting.write.transferVesting([alice.account.address, bob.account.address, parseEther("1000")], { account: owner.account }))
          .to.rejectedWith("SS_StepVesting: insufficient balance");          
        })

Run the test and it succeeds.

Recommended mitigation steps

Consider adding trusted role to unlist from marketplace, so that the reallocation can be handle completely