merlin - The claim function in PerAddressContinuousVestingMerkle.sol will always fail due to incorrect decoding

[sherlock-admin4]

merlin

medium

The claim function in PerAddressContinuousVestingMerkle.sol will always fail due to incorrect decoding

Summary

The same issue is present in both the PerAddressTrancheVestingMerkleDistributor and PerAddressContinuousVestingMerkleDistributor smart contracts.

Vulnerability Detail

If a user is a beneficiary in merkle root, they can call the PerAddressContinuousVestingMerkle.claim function. Let's break down the claim function step-by-step:

function claim(
    /// arguments
  )
    external
    validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, totalAmount, start, cliff, end)), merkleProof)
    nonReentrant
  {
    // effects
--> uint256 claimedAmount = _executeClaim(beneficiary, totalAmount, new bytes(0));
    // interactions
    _settleClaim(beneficiary, claimedAmount);
  }

The AdvancedDistributor smart contract calls the _executeClaim function in the Distributor smart contract, which in turn calls getClaimableAmount:

/// AdvancedDistributor
 function _executeClaim(
    address beneficiary,
    uint256 totalAmount,
    bytes memory data
  ) internal virtual override returns (uint256 _claimed) {
--> _claimed = super._executeClaim(beneficiary, totalAmount, data);
    _reconcileVotingPower(beneficiary);
  }

/// Distributor  
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');

    /// code
  }  

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
  }  

getClaimableAmount calls getVestedFraction, which decodes the data parameter (new bytes(0)), leading to the transaction failure:

function getVestedFraction(
        address beneficiary,
        uint256 time, // time is in seconds past the epoch (e.g. block.timestamp)
    bytes memory data
    ) public view override returns (uint256) {
--> (uint256 start, uint256 cliff, uint256 end) = abi.decode(data, (uint256, uint256, uint256));

///code

Impact

The beneficiary is unable to claim tokens.

Code Snippet

contracts/claim/abstract/PerAddressContinuousVesting.sol#L25

Tool used

Manual Review

Recommendation

Consider passing the data parameter not as new bytes(0), but rather as encoded start, cliff, and end periods.

Duplicate of #11