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

2 stars 1 forks source link

BiasedMerc - AdvancedDistributorInitializable::_executeClaim() doesn't allow to pass custom data parameter #38

Closed sherlock-admin3 closed 4 weeks ago

sherlock-admin3 commented 1 month ago

BiasedMerc

medium

AdvancedDistributorInitializable::_executeClaim() doesn't allow to pass custom data parameter

Summary

AdvancedDistributorInitializable::_executeClaim() doesn't allow to pass custom data parameter, which will lead to reverts when inherited.

Vulnerability Detail

AdvancedDistributorInitializable::_executeClaim()

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

The data field when calling super._executeClaim is set to new bytes(0).

PerAddressTrancheVestingInitializable inherits AdvancedDistributorInitializable and contains the following PerAddressTrancheVestingInitializable::getVestedFraction() function:

    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;
            }
        }

        return 0;
    }

As seen, the data parameter is utilised however it will be passed as bytes(0) meaning it will not function as intended and will be incorrectly decoded leading to a revert.

Impact

The inability to pass the data parameter into _executeClaim will lead to the function to not work as intended, leading to a breaking of functionality.

Code Snippet

AdvancedDistributorInitializable::_executeClaim() PerAddressTrancheVestingInitializable::getVestedFraction()

Tool used

Manual Review

Recommendation

Do not set the data parameter to bytes(0) and instead utilise the passed data parameter, e.g:

AdvancedDistributorInitializable::_executeClaim()

-    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