hats-finance / Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573

0 stars 1 forks source link

No upper and lower fee checks in `WiseLowLevelHelper.setPoolFee()` #43

Open hats-bug-reporter[bot] opened 9 months ago

hats-bug-reporter[bot] commented 9 months ago

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xf0e6aab6daf36b8f3530323fbf0be5e103a0125398b5b0d3a0605cd5ca8fe3c5 Severity: medium

Description: Description\

This is actually a low severity issue submitted with the intent the protocol team should fix in contracts.

WiseLowLevelHelper.setPoolFee() is used to set the pool fee or to update the pool with new fee. The fee Manager is only allowed to set the


    function setPoolFee(
        address _poolToken,
        uint256 _newFee
    )
        external
        onlyFeeManager
    {
        globalPoolData[_poolToken].poolFee = _newFee;
    }

The issue is, this function does not have fee limit checks i.e there is no lower or higher fee limits are checked. newFee is uint256 input and the value can be passed as high or as low as zero.

Fee Manager should be restricted to set the _newFee within limits and it should be checked in function.

Recommendation\

Enforce the upper and lower fee checks in WiseLowLevelHelper.setPoolFee().

0xRizwan commented 9 months ago

For example, USDT has a hardcoding limit beyond which fees can never be added and it ensure transparency.

    function setParams(uint newBasisPoints, uint newMaxFee) public onlyOwner {
        // Ensure transparency by hardcoding limit beyond which fees can never be added
        require(newBasisPoints < 20);
        require(newMaxFee < 50);

        basisPointsRate = newBasisPoints;
        maximumFee = newMaxFee.mul(10**decimals);

        Params(basisPointsRate, maximumFee);
    }
vm06007 commented 9 months ago

seems like decentralization topic on what is allowed to be passed by FeeManager. Keep in mind FeeManager can be a contract that can have this check done in a wrapper function that is calling setPoolFee() on the main contract.