0xboriskataa - 0 data input might break functionality

[sherlock-admin3]

0xboriskataa

medium

0 data input might break functionality

Summary

0 data input might break functionality

Vulnerability Detail

In the scope there are contracts that have a default implementation and an initializable one. The only difference between the two should be that the initializable one should follow the upgradability pattern. However AdvancedDistributor.sol & AdvancedDistributorInitializable.sol have different function parameters:

AdvancedDistributor.sol:

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

AdvancedDistributorInitializable.sol:

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

As you can see the second one calls the super._executeClaim with 0 data instead of using the data from the input.

Impact

I'm not sure what effect this might have but the other contracts that have an initializable implementation only use the upgradability pattern and don't change anything else in the code. Perhaps this might break some functionality as some functions require data to be used.

Here is how the execution continues after calling super._executeClaim: _executeClaim() -> getClaimableAmount() -> getVestedFraction()

getVestedFraction() is in most of the files so it's confusing to determine which implementation actually gets called. But here is one of the implementations of getVestedFraction() that requires data in order to work:

    function getVestedFraction(address beneficiary, uint256 time, bytes memory data) public view override returns (uint256) {
        Tranche[] memory tranches = abi.decode(data, (Tranche[]));

        uint256 delay = getFairDelayTime(beneficiary);
        for (uint256 i = tranches.length; i != 0;) {
            unchecked {
                --i;
            }

            if (time - delay > tranches[i].time) {
                return tranches[i].vestedFraction;
            }
        }

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/abstract/AdvancedDistributor.sol#L107

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

Use consistent code when implementing initializable versions of contracts.

Duplicate of #11