code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

Users are not able to withdraw funds if strategy incurs a loss #720

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchPort.sol#L144

Vulnerability details

Impact

While depositing underlyingTokens in the system users receive hTokens in return. The whole system assumes that the ratio between these hTokens and underlyingTokens is always 1:1. This would hold true if there wasn't the ability to utilize underlyingTokens in strategies. In case the strategy incurs a loss the system will end up in a situation whereby some users cannot withdraw their funds. The likelihood of this happening is medium to low since the strategies will be vetted by governance. Nevertheless, even the most secure strategies can experience hacks or exploits. The system in its current state does not have a trustless mechanism to handle this situation.

Let's imagine a scenario whereby a popular protocol gets hacked, e.g. Curve. The hack affects the Maia DAO as well, and the whole system is in a debt state. The most sophisticated users are quick to exit the system and withdraw their funds, but the rest of the users are left with a loss.

Recommended Mitigation Steps

A broader architectural change would be making hTokens ERC4626 vaults instead of 1:1 wrappers. This way all the incurred losses would be distributed to all the users proportionally.

A simpler solution would be to have governance the ability to pause the system and not allow sophisticated users to withdraw their funds until the system is in a healthy state again. This way at least all the users would be treated equally in case of a bank run.

Assessed type

Other

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

0xLightt commented 1 year ago

Strategies need to be vetted by governance. Making hTokens ERC4626 vaults instead of 1:1 wrappers would increase gas costs significantly and being new code may introduce another flaws. Governance would add to centralization risk and availability concerns.

c4-sponsor commented 1 year ago

0xLightt (sponsor) acknowledged

c4-sponsor commented 1 year ago

0xLightt marked the issue as disagree with severity

alcueca commented 1 year ago

This report should have been submitted as an analysis

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid