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

3 stars 2 forks source link

aman - The `new bytes(0)` will result in revert for claim function #45

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

aman

high

The new bytes(0) will result in revert for claim function

Summary

The PerAddressContinuousVestingMerkleDistributor and PerAddressTrancheVestingMerkleDistributor contract will always revert due to new bytes(0) pass to _executeClaim function.

Vulnerability Detail

When user tries to claim the Token from PerAddressContinuousVestingMerkleDistributor and PerAddressTrancheVestingMerkleDistributor the user will calls the claim() function :

2024-05-tokensoft-distributor-contracts-update/contracts/packages/hardhat/contracts/claim/factory/PerAddressContinuousVestingMerkleDistributor.sol:52
52:     function claim(
53:         uint256 index, // the beneficiary's index in the merkle root
54:         address beneficiary, // the address that will receive tokens
55:         uint256 totalAmount, // the total claimable by this beneficiary
56:         uint256 start, // the start of the vesting period
57:         uint256 cliff, // cliff time
58:         uint256 end, // the end of the vesting period
59:         bytes32[] calldata merkleProof
60:     )
61:         external
62:         validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, totalAmount, start, cliff, end)), merkleProof)
63:         nonReentrant
64:     {
65:         // effects
66:         uint256 claimedAmount = super._executeClaim(beneficiary, totalAmount, new bytes(0)); // <-----
67:         // interactions
68:         _settleClaim(beneficiary, claimedAmount);
69:     }

Let's have a look at this _executeClaim which will be called with this chain of contracts : PerAddressContinuousVestingMerkleDistributor > AdvancedDistributorInitializable > DistributorInitializable:

2024-05-tokensoft-distributor-contracts-update/contracts/packages/hardhat/contracts/claim/factory/DistributorInitializable.sol:66
66:     function _executeClaim(address beneficiary, uint256 _totalAmount, bytes memory data) internal virtual returns (uint256) {
67:         uint120 totalAmount = uint120(_totalAmount);
68: 
69:         // effects
70:         if (records[beneficiary].total != totalAmount) {
71:             // re-initialize if the total has been updated
72:             _initializeDistributionRecord(beneficiary, totalAmount);
73:         }
75: 
76: 
77:         uint120 claimableAmount = uint120(getClaimableAmount(beneficiary, data));
79:         require(claimableAmount > 0, "Distributor: no more tokens claimable right now");
80: 
81:         records[beneficiary].claimed += claimableAmount;
82:         claimed += claimableAmount;
83: 
84:         return claimableAmount;
85:     }

Now let check getVestedFraction which is called in getClaimableAmount function as we are in PerAddressContinuousVestingMerkleDistributor contract :

2024-05-tokensoft-distributor-contracts-update/contracts/packages/hardhat/contracts/claim/factory/PerAddressContinuousVestingInitializable.sol:30
30:     function getVestedFraction(
31:         address beneficiary,
32:         uint256 time, // time is in seconds past the epoch (e.g. block.timestamp)
33:         bytes memory data
34:     ) public view override returns (uint256) {
35:         (uint256 start, uint256 cliff, uint256 end) = abi.decode(data, (uint256, uint256, uint256));
36: 
37:         uint256 delayedTime = time - getFairDelayTime(beneficiary);
38:         // no tokens are vested
39:         if (delayedTime <= cliff) {
40:             return 0;
41:         }
42: 
43:         // all tokens are vested
44:         if (delayedTime >= end) {
45:             return fractionDenominator;
46:         }
47: 
48:         // some tokens are vested
49:         return (fractionDenominator * (delayedTime - start)) / (end - start);
50:     }

And in case of Tranches:

2024-05-tokensoft-distributor-contracts-update/contracts/packages/hardhat/contracts/claim/factory/PerAddressTrancheVestingInitializable.sol:37
37:     function getVestedFraction(address beneficiary, uint256 time, bytes memory data) public view override returns (uint256) {
38:         // data is 0 here so it will always return 0
39:         Tranche[] memory tranches =  abi.decode(data, (Tranche[]));
40: 
41:         uint256 delay = getFairDelayTime(beneficiary);
42:         for (uint256 i = tranches.length; i != 0;) {
43:             unchecked {
44:                 --i;
45:             }
46:             if (time - delay > tranches[i].time) {
47:                 return tranches[i].vestedFraction;
48:             }
49:         }
50:         return 0; // will return 0 here
51:     }

As we pass new bytes(0) this function will always reverts.

test case please add this code to TrancheVestingMerkleDistributorFactoryTest: add import :

import "../../contracts/claim/factory/PerAddressTrancheVestingMerkleDistributor.sol";
import "../../contracts/claim/factory/PerAddressTrancheVestingMerkleDistributorFactory.sol";
function testClaimRevert() external {
        factory1 = new PerAddressTrancheVestingMerkleDistributorFactory(
            address(new PerAddressTrancheVestingMerkleDistributor())
        );
        implementation1 = factory1.deployDistributor(
            IERC20(token),
            1000,
            "uri",
            bytes32(
                0xf32ce147ef6c8e7a07fbe3ce1ae1aef2405a59b50db2a8ede14f4e75bfe7d949
            ),
            10,
            address(this),
            1
        );
        implementation1.setToken(IERC20(token));
        bytes32[] memory proof = new bytes32[](1);
        token.transfer(address(implementation1), 10e18);
        proof[0] = bytes32(
            0xa4170e52dc35c1127b67e1c2f5466fce9673e61b44e077b7023f7c994d55c5dd
        );
        implementation1.claim(
            1,
            address(0x70997970C51812dc3A010C7d01b50e0d17dc79C8),
            5000000000000000000000,
            proof
        );
    }

run this test case with : forge test --mt testClaimRevert.

Impact

Due to new bytes(0) the claim function will always reverts. and users will not be able to claim there tokens.

Code Snippet

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

Tool used

Manual Review

Recommendation

Check Again and add required changes for extra data required while claiming tokens.

Duplicate of #11