kennedy1030 - Modifying the `_managerFeePIPS` variable within the `ValantisHOTModule` is not possible until the `ValantisHOTModule` has been designated as the `poolManager` of the SovereignPool.

[sherlock-admin2]

kennedy1030

medium

Modifying the _managerFeePIPS variable within the ValantisHOTModule is not possible until the ValantisHOTModule has been designated as the poolManager of the SovereignPool.

Summary

Modifying the _managerFeePIPS variable within the ValantisHOTModule triggers the execution of the SovereignPool::setPoolManagerFeeBips function. However, this operation will fail as long as the ValantisHOTModule has not been designated as the poolManager of the SovereignPool, since the SovereignPool::setPoolManagerFeeBips function can only be called by the poolManager.

Vulnerability Detail

If the ValantisHOTModule::setManagerFeePIPS function is invoked to modify the _managerFeePIPS value, it will trigger a call to the SovereignPool::setPoolManagerFeeBips function, even though the alm may not be set yet, as the _oldFee value is still present.

However, the SovereignPool::setPoolManagerFeeBips function is protected by the onlyPoolManager modifier. Therefore, if the ValantisHOTModule has not been designated as the poolManager of the SovereignPool, the transaction attempting to update the manager fee bips will fail.

    function setManagerFeePIPS(uint256 newFeePIPS_)
        external
        whenNotPaused
    {
        uint256 _oldFee = _managerFeePIPS;

        // #region checks.

        if (msg.sender != metaVault.manager()) {
            revert OnlyManager(msg.sender, metaVault.manager());
        }

        if (newFeePIPS_ > PIPS) revert NewFeesGtPIPS(newFeePIPS_);

        // #endregion checks.

        _managerFeePIPS = newFeePIPS_;

289     if (address(alm) != address(0) || _oldFee != 0) {
290         pool.setPoolManagerFeeBips(newFeePIPS_ / 1e2);
        }

        emit LogSetManagerFeePIPS(_oldFee, newFeePIPS_);
    }

Impact

Modifying the poolManagerFeeBips value of the SovereignPool is not possible until the ValantisHOTModule has been designated as the poolManager of the SovereignPool.

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/abstracts/ValantisHOTModule.sol#L271-L294

Tool used

Manual Review

Recommendation

The ValantisHOTModule::setManagerFeePIPS function should be fixed as follows.

    function setManagerFeePIPS(uint256 newFeePIPS_)
        external
        whenNotPaused
    {
        uint256 _oldFee = _managerFeePIPS;

        // #region checks.

        if (msg.sender != metaVault.manager()) {
            revert OnlyManager(msg.sender, metaVault.manager());
        }

        if (newFeePIPS_ > PIPS) revert NewFeesGtPIPS(newFeePIPS_);

        // #endregion checks.

        _managerFeePIPS = newFeePIPS_;

-       if (address(alm) != address(0) || _oldFee != 0) {
+       if (address(alm) != address(0) && _oldFee != 0) {
            pool.setPoolManagerFeeBips(newFeePIPS_ / 1e2);
        }

        emit LogSetManagerFeePIPS(_oldFee, newFeePIPS_);
    }