sherlock-audit / 2024-05-tokensoft-distributor-contracts-update-judging

2 stars 1 forks source link

hunter_w3b - Claim Function Fails in `PerAddressTrancheVestingMerkleDistributor` Due to Empty Data #39

Closed sherlock-admin4 closed 4 weeks ago

sherlock-admin4 commented 1 month ago

hunter_w3b

high

Claim Function Fails in PerAddressTrancheVestingMerkleDistributor Due to Empty Data

Summary

When users attempt to claim their tokens by calling the PerAddressTrancheVestingMerkleDistributor::claim function, the function internally calls _executeClaim with an empty byte array new bytes(0). This causes the getVestedFraction function to always return zero, leading to the claim always reverting.

Vulnerability Detail

When users attempt to claim their tokens by calling the PerAddressTrancheVestingMerkleDistributor::claim function, the function internally calls _executeClaim with an empty byte array (new bytes(0)).

    //..
    {
        // effects
@>>        uint256 claimedAmount = _executeClaim(beneficiary, totalAmount, new bytes(0));
        // interactions
        _settleClaim(beneficiary, claimedAmount);
    }

AdvancedDistributorInitializable ---> DistributorInitializable inside DistributorInitializable the _executeClaim function calls getClaimableAmount with the empty byte array.

    function getClaimableAmount(address beneficiary, bytes memory data) public view virtual returns (uint256) {
        require(records[beneficiary].initialized, "Distributor: claim not initialized");

        DistributionRecord memory record = records[beneficiary];

        //  @audit  getVestedFraction always return zero  because of data
@>>        uint256 claimable = (record.total * getVestedFraction(beneficiary, block.timestamp, data)) / fractionDenominator;
        return record.claimed >= claimable
            ? 0 // no more tokens to claim
            : claimable - record.claimed; // claim all available tokens
    }

Based on the implementation of PerAddressTrancheVestingInitializable::getVestedFraction function is always returning zero. This issue arises because the data parameter passed to the function has a zero length. As a result, the function fails to decode any tranches and consequently never finds a valid vestedFraction and return 0.

getVestedFraction tries to decode vesting tranches from the data, but since the data is empty, the length of the tranches array is zero. Therefore, the loop does not execute, and the function returns zero.

    function getVestedFraction(address beneficiary, uint256 time, bytes memory data) public view override returns (uint256) {
@>>        Tranche[] memory tranches = abi.decode(data, (Tranche[]));

        uint256 delay = getFairDelayTime(beneficiary);
        for (uint256 i = tranches.length; i != 0;) {
            unchecked {
                --i;
            }

            if (time - delay > tranches[i].time) {
                return tranches[i].vestedFraction;
            }
        }

        return 0;
    }

So, the value of getClaimableAmount is zero, then _executeClaim checks if the claimable amount is greater than zero. Since it is not, the function reverts."

    function _executeClaim(address beneficiary, uint256 _totalAmount, bytes memory data) internal virtual returns (uint256) {
        uint120 totalAmount = uint120(_totalAmount);

        // effects
        if (records[beneficiary].total != totalAmount) {
            // re-initialize if the total has been updated
            _initializeDistributionRecord(beneficiary, totalAmount);
        }

        uint120 claimableAmount = uint120(getClaimableAmount(beneficiary, data));
        // @audit claimableAmount is always zero revert
        require(claimableAmount > 0, "Distributor: no more tokens claimable right now");

        records[beneficiary].claimed += claimableAmount;
        claimed += claimableAmount;

        return claimableAmount;
    }

Simple Scenario

  1. The owner deploys the PerAddressTrancheVestingMerkleDistributor Distributor contract and initializes it for token distribution, setting up various tranches for vesting.
  2. Users are informed that they can call the claim function to receive their vested tokens.
  3. Alice Initializes Her Distribution Record.
  4. Alice follows the instructions and calls the claim function, expecting to receive her vested tokens.
  5. Internally, the claim function calls _executeClaim with an empty byte array as the data parameter.
  6. The _executeClaim function attempts to determine the claimable amount by calling getClaimableAmount, which in turn calls getVestedFraction.
  7. Since getVestedFraction is called with an empty data parameter, it decodes no tranches and returns a vested fraction of zero.
  8. As a result, getClaimableAmount returns zero, indicating no tokens are claimable.
  9. The require statement in _executeClaim fails because the claimable amount is zero, causing the transaction to revert.
  10. Alice's attempt to claim her vested tokens fails, and she receives no tokens.

Impact

This issue affects all users attempting to claim their tokens, rendering the token distribution system non-functional.

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/PerAddressTrancheVestingMerkleDistributor.sol#L51-L65

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/AdvancedDistributorInitializable.sol#L106-L114

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/DistributorInitializable.sol#L66-L82

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/PerAddressTrancheVestingInitializable.sol#L37-L52

Tool used

Manual Review

Recommendation

To fix this issue, the _executeClaim function should pass the correct data parameter containing the vesting tranches to the getClaimableAmount function.

Duplicate of #11