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

2 stars 1 forks source link

samuraii77 - Owner funds could get locked #44

Closed sherlock-admin3 closed 4 weeks ago

sherlock-admin3 commented 1 month ago

samuraii77

medium

Owner funds could get locked

Summary

Owner funds could get locked

Vulnerability Detail

The owner can call AdvancedDistributorInitializable#adjust() in order to adjust the amount that a user can claim:

    function adjust(address beneficiary, int256 amount) external onlyOwner {
        DistributionRecord memory distributionRecord = records[beneficiary];
        require(distributionRecord.initialized, "must initialize before adjusting");

        uint256 diff = uint256(amount > 0 ? amount : -amount);
        require(diff < type(uint120).max, "adjustment > max uint120");

        if (amount < 0) {
            // decreasing claimable tokens
            require(total >= diff, "decrease greater than distributor total");
            require(distributionRecord.total >= diff, "decrease greater than distributionRecord total");
            total -= diff;
            records[beneficiary].total -= uint120(diff);
            token.safeTransfer(owner(), diff);
        } else {
            // increasing claimable tokens
            total += diff;
            records[beneficiary].total += uint120(diff);
        }
        _reconcileVotingPower(beneficiary);
        emit Adjust(beneficiary, amount);
    }

Upon deployment, the recipient of the Sweepable contract is the msg.sender. It can also later be changed to any other address by the owner. The issue here is that if the total allocated tokens are decreased in adjust() function, the tokens are sent to the owner. That means that the recipient of those funds can be a completely different address than the one set as recipient. This is extremely illogical and could cause loss of funds in cases where the owner is a contract with no way of getting out tokens or if it is an EOA that was just used for deploying and was then left behind. The protocols integrating TokenSoft would expect that since they set the recipient, that would be the address that receives the tokens.

Impact

Owner funds could get locked

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/factory/AdvancedDistributorInitializable.sol#L124-L145

Tool used

Manual Review

Recommendation

Use pull over push or send the tokens to the recipient address.