There is a chance that the beneficiary won't be able to claim the rest of his tokens

Summary

Instead of the beneficiary being able to claim the rest of his available tokens he won't be able to.

Vulnerability Detail

When the _executeClaim function is called for the beneficiary. This is how this function is represented:

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));
    require(claimableAmount > 0, 'Distributor: no more tokens claimable right now');

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

    return claimableAmount;
  }

As we can see the claimableAmount variable is set by calling the getClaimableAmount function. Which is represented like this:

 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];

    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
  }

Now let's say that the beneficiary had claimed before and the record.claimed has a value of 100e18 for example. After some time when the _executeClaim function is called again and the claimableAmount needs to be calculated, and this math happens uint256 claimable = (record.total * getVestedFraction(beneficiary, block.timestamp, data)) / fractionDenominator;. And if the result is less than 100e18, let's say 50e18. The value that the getClaimableAmount function will return will be 0 because the claimable will be less than the record.claimed. This will happen because of this line of code:

 return
      record.claimed >= claimable
        ? 0 // no more tokens to claim
        : claimable - record.claimed; // claim all available tokens

Instead of the beneficiary being able to claim those available tokens he will lose them, because when the claimableAmount variable is set to 0 in the _executeClaim function this function will revert because of this line of code:

 require(claimableAmount > 0, 'Distributor: no more tokens claimable right now');

Impact

The beneficiary can have his available tokens for claiming set to 0 instead of him being able to claim them.

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/abstract/Distributor.sol#L66-L86

Tool used

Manual Review

Recommendation

Remove the check which checks if the record.claimed is more than or equal to claimable.

Duplicate of #2

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

fandonov - There is a chance that the beneficiary won't be able to claim the rest of his tokens #7