Ironsidesec - PerAddressContinuousVestingMerkle.claim will always revert

[sherlock-admin3]

Ironsidesec

high

PerAddressContinuousVestingMerkle.claim will always revert

Summary

Impact : beneficiary can never claim the tokens from PerAddressContinuousVestingMerkle contract
Likelihood : always, and issue also exists on PerAddressContinuousVestingMerkleDistributor.claim(), PerAddressTrancheVestingMerkleDistributor.claim(). But doesn't exist on PerAddressTrancheVesting.claim() because it is encoding the data properly

Root cause : start, cliff, end values are not encoded when computing getVestedFraction and it will revert when it tries to decode the zero bytes data into start, cliff, end

Vulnerability Detail

Issue flow :

1. To claim the vested tokens , beneficiary should call PerAddressContinuousVestingMerkle.claim() with start, cliff, end to verify with merkle root. Look at line 68, it is sending new bytes(0) instead of abi.encode(start, cliff, end)

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/PerAddressContinuousVestingMerkle.sol#L67

2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\PerAddressContinuousVestingMerkle.sol

53:   function claim(
54:     uint256 index, // the beneficiary's index in the merkle root
55:     address beneficiary, // the address that will receive tokens
56:     uint256 totalAmount, // the total claimable by this beneficiary
57:     uint256 start, // the start of the vesting period
58:     uint256 cliff, // cliff time
59:     uint256 end, // the end of the vesting period
60:     bytes32[] calldata merkleProof
61:   )
62:     external
63:     validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, totalAmount, start, cliff, end)), merkleProof)
64:     nonReentrant

... SNIP ...

68: >>> uint256 claimedAmount = _executeClaim(beneficiary, totalAmount, new bytes(0));
69:     // interactions
70:     _settleClaim(beneficiary, claimedAmount);

... SNIP ...

2. _executeClaim on line 68 above calls L110 below and data = new bytes(0) is passed in L115 below.

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

2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\abstract\AdvancedDistributor.sol

110:   function _executeClaim(
... SNIP ...

114:   ) internal virtual override returns (uint256 _claimed) {
115: >>> _claimed = super._executeClaim(beneficiary, totalAmount, data);
116:     _reconcileVotingPower(beneficiary);
117:   }

3. L115 above calls L66 below with data = new bytes(0) and it will call getClaimableAmount internall. in L124 below it is again calling getVestedFraction internally to compute how much is vested based on current delayed time between start, cliff, end

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

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

2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\abstract\Distributor.sol

66:   function _executeClaim(
67:     address beneficiary,
68:     uint256 _totalAmount,
69:     bytes memory data
70:   ) internal virtual returns (uint256) {
... SNIP ...
79:     
80: >>> uint120 claimableAmount = uint120(getClaimableAmount(beneficiary, data));
81:     require(claimableAmount > 0, 'Distributor: no more tokens claimable right now');
82: 
... SNIP ...
87:   }

119:   function getClaimableAmount(address beneficiary, bytes memory data) public view virtual returns (uint256) {
... SNIP ...
123: 
124: >>>   uint256 claimable = (record.total * getVestedFraction(beneficiary, block.timestamp, data)) /
125:       fractionDenominator;
... SNIP ...
130:   }

4. In L25 below the data is decoded into start, cliff, end, since we sent data = new bytes(0) from starting, this will trigger the revert.
   https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/abstract/PerAddressContinuousVesting.sol#L25

2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\abstract\PerAddressContinuousVesting.sol

20: 	function getVestedFraction(
21: 		address beneficiary,
22: 		uint256 time, // time is in seconds past the epoch (e.g. block.timestamp)
23:  >>>    bytes memory data
24: 	) public view override returns (uint256) {
25:  >>> (uint256 start, uint256 cliff, uint256 end) = abi.decode(data, (uint256, uint256, uint256));
26: 
... SNIP ...
43: 		// some tokens are vested
44: 		return (fractionDenominator * (delayedTime - start)) / (end - start);
45: 	}

Impact

Impact : beneficiary can never claim the tokens from PerAddressContinuousVestingMerkle contract
Likelihood : always, and issue also exists on PerAddressContinuousVestingMerkleDistributor.claim(), PerAddressTrancheVestingMerkleDistributor.claim(). But doesn't exist on PerAddressTrancheVesting.claim() because it is encoding the data properly

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/PerAddressContinuousVestingMerkle.sol#L67

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

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

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

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

Tool used

Manual Review

Recommendation

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/PerAddressContinuousVestingMerkle.sol#L67

  function claim(
    uint256 index, // the beneficiary's index in the merkle root
    address beneficiary, // the address that will receive tokens
    uint256 totalAmount, // the total claimable by this beneficiary
    uint256 start, // the start of the vesting period
    uint256 cliff, // cliff time
    uint256 end, // the end of the vesting period
    bytes32[] calldata merkleProof
  )
    external
    validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, totalAmount, start, cliff, end)), merkleProof)
    nonReentrant
  {
    // effects
-   uint256 claimedAmount = _executeClaim(beneficiary, totalAmount, new bytes(0));
+   uint256 claimedAmount = _executeClaim(beneficiary, totalAmount, abi.encode(start, cliff, end));
    // interactions
    _settleClaim(beneficiary, claimedAmount);
  }

Duplicate of #11