Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0x52 - MorphoLeverageModule#removeModule is broken and cannot be used without trapping funds #37

Open
sherlock-admin3 opened this issue Oct 21, 2024 · 0 comments

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Oct 21, 2024

0x52

Medium

MorphoLeverageModule#removeModule is broken and cannot be used without trapping funds

Summary

Removing a module for a set token is a core functionality of the protocol but MorphoLeverageModule cannot be removed without breaking set token redemption and trapping user funds. The standard methodology is that MorphoLeverageModule#removeModule would be called after the set has deleveraged to zero debt. However it fails to withdraw the collateral from Morpho, resulting in the funds becoming inaccessible after removal.

This also violates the expected behavior of the module based on comments in the contract itself and the set token

Root Cause

MorphoLeverageModule#removeModule fails to withdraw collateral from Morpho

Internal pre-conditions

None. Funds are trapped regardless of amount.

External pre-conditions

None.

Attack Path

SetToken.sol#L376-L387

function removeModule(address _module) external onlyManager {
    require(!isLocked, "Only when unlocked");
    require(moduleStates[_module] == ISetToken.ModuleState.INITIALIZED, "Module must be added");

    IModule(_module).removeModule(); <-- @audit call to MorphoLeverageModule

    moduleStates[_module] = ISetToken.ModuleState.NONE;

    modules.removeStorage(_module);

    emit ModuleRemoved(_module);
}

The removal process begins with the set token which calls MorphoLeverageModule#removeModule.

MorphoLeverageModule.sol#L446-L462

function removeModule()
    external
    override
    onlyValidAndInitializedSet(ISetToken(msg.sender))
{
    ISetToken setToken = ISetToken(msg.sender);

    sync(setToken);

    delete marketParams[setToken];

    // Try if unregister exists on any of the modules
    address[] memory modules = setToken.getModules();
    for(uint256 i = 0; i < modules.length; i++) {
        try IDebtIssuanceModule(modules[i]).unregisterFromIssuanceModule(setToken) {} catch {} <-- @audit unregisters but never withdraws funds from morpho
    }
}

This unregisters the MorphoLeverageModule from the debtIssuanceModule and clears the marketParams.

DebtIssuanceModule.sol#L288-L292

function unregisterFromIssuanceModule(ISetToken _setToken) external onlyModule(_setToken) onlyValidAndInitializedSet(_setToken) {
    require(issuanceSettings[_setToken].isModuleHook[msg.sender], "Module not registered.");
    issuanceSettings[_setToken].moduleIssuanceHooks.removeStorage(msg.sender);
    issuanceSettings[_setToken].isModuleHook[msg.sender] = false;
}

This in turn clears the issuance/redemption hooks, preventing the assets from being accessed.

Impact

When module is removed, set token redemption breaks and user funds are trapped.

PoC

No response

Mitigation

MorphoLeverageModule#removeModule should withdraw all funds from Morpho and update the default position for the collateral component.

@sherlock-admin2 sherlock-admin2 changed the title Polite Sand Finch - MorphoLeverageModule#removeModule is broken and cannot be used without trapping funds 0x52 - MorphoLeverageModule#removeModule is broken and cannot be used without trapping funds Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant