Honour - `AdvancedDistributorInitializable::_executeClaim()` ignores the encoded vesting periods data

[sherlock-admin2]

Honour

high

AdvancedDistributorInitializable::_executeClaim() ignores the encoded vesting periods data

Summary

AdvancedDistributorInitializable::_executeClaim() ignores encoded vesting data parameter and instead passes an empty bytes array to be used for claiming calculations and as a result DOSing the claiming process.

Vulnerability Detail

AdvancedDistrubutionInitializable::_executeClaim() ignores the encoded vesting periods data parameter used to calculate the amount of tokens claimable and instead passes an empty bytes array to Distributor::_executeClaim() to be used for claiming calculations. The issues is that claiming reverts on getVestedFraction() on decoding said vesting data , as a result DOSing the claiming process.

POC

Add a Mock PerAddressContinuousVestingMerkle.sol to bypass merkle proof validation.

contract MockPACVMD is PerAddressContinuousVestingMerkleDistributor {
    function mock_initializeDistributionRecord(
        uint256, //index, // the beneficiary's index in the merkle root
        address beneficiary, // the address that will receive tokens
        uint256 amount, // the total claimable by this beneficiary
        bytes32[] calldata //merkleProof
    ) external {
        _initializeDistributionRecord(beneficiary, amount);
    }

    function mock_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 startTime, // the start time of the claim
        uint256 cliffTime, // the cliff time of the claim
        uint256 endTime, // the end time of the claim
        bytes32[] calldata // merkleProof
    ) external nonReentrant {
        // effects
        uint256 claimedAmount = super._executeClaim(
            beneficiary,
            totalAmount,
            abi.encode(startTime, cliffTime, endTime)
        );
        // interactions
        _settleClaim(beneficiary, claimedAmount);
    }

}

create a test file and add the following to it and necessary imports. run forge test --mt test_claim

contract PerAddressContinuousVestingMerkleDistributorFactoryTest is Test {
    MockPACVMD implementation;
    MockPACVMD clone;
    PerAddressContinuousVestingMerkleDistributorFactory factory;
    ERC20 token = new ERC20("Test", "TEST");

    function setUp() public {
        implementation = new MockPACVMD();
        factory = new PerAddressContinuousVestingMerkleDistributorFactory(
            address(implementation)
        );
        clone = MockPACVMD(address(
            factory.deployDistributor(
                IERC20(token),
                1000,
                "uri",
                bytes32(0),
                0,
                address(this),
                0
            )
        ));
    }

    function test_claim() public {
        address alice = makeAddr("alice");

    //initialize alice's distribution record
        clone.mock_initializeDistributionRecord(
            0,
            alice,
            1 ether,
            new bytes32[](0)
        );

    //fund clone with distribution tokens
        deal(address(token), address(clone), 10 ether);

        //advance time
    vm.warp(100);

        vm.expectRevert();
        clone.mock_claim(
            0,
            alice,
            3 ether,
            block.timestamp - 50,
            block.timestamp - 50,
            block.timestamp + 50,
            new bytes32[](0)
        );

    }
}

Impact

High, permanent DOS of claim, users will not be able to claim their tokens

Code Snippet

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

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

Tool used

Manual Review

Recommendation

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

Duplicate of #11