[H-2] Fee manager's funds gets stuck in contract

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PositionNFTs.sol#L71-L88

Vulnerability details

Description:

Fee manager's funds get stuck in smart contract after the first transfer.

Impact:

Loss of funds meant to be paid as fees to the feeManager

Tools Used

Manual Review

Proof Of Concept:

Upon declaration the feeManger address is equal to zero, which passes the "if" check and sends the feeManager a FEE_MANAGER_NFT. subsequent transfer will fail the "if" check and revert "NotPermitted()" because the feeManager address will now be initialized with the _feeManagerContract which is a dynamic address passed as a parameter to the `forwardFeeManagerNFT.

    function forwardFeeManagerNFT(
        address _feeManagerContract
    )
        external
        onlyMaster
    {
        if (feeManager > ZERO_ADDRESS) {
            revert NotPermitted();
        }

        feeManager = _feeManagerContract;

        _transfer(
            address(this),
            _feeManagerContract,
            FEE_MANAGER_NFT
        );
    }

Recommended Mitigation Steps:

Re configure the logic and set feeManager address back to zero after every successfull _transfer().

    function forwardFeeManagerNFT(
        address _feeManagerContract
    )
        external
        onlyMaster
    {
        if (feeManager > ZERO_ADDRESS) {
            revert NotPermitted();
        }

        feeManager = _feeManagerContract;

        _transfer(
            address(this),
            _feeManagerContract,
            FEE_MANAGER_NFT
        );

+       feeManager = address(0);

    }

Assessed type

Token-Transfer

[vm06007]

suggested code edit does not make sense. if feeManager is set to 0x0 as suggested then all fees would go to burn address. please read code carefully and understand why feeManager must be a legit value and should not be set to 0x0 address as proposed.

setting feeManager should be allowed only once that is why input is compared to > ZERO_ADDRESS

this submission can be dismissed and marked as invalid

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid