c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The minimum token reserve ratio getMinimumTokenReserveRatio can only be set once per underlying token per chain and can never be changed. In the event of a significant market issue like a vulnerability, hack or depeg user's will seek to bridge back their htokens for the original underlying asset creating a rush on liquidity in Branch Ports. At the same time Port Strategies are using Branch Ports as a source of liquidity for their strategies. Not having a governance function to increase reserves to match market conditions and the liquidity requirements of users will cause a significant economic problem for the protocol during significant events like hacks, crashes or depegs.

Further exascerbating the issue is there's nothing to compel Port Strategies to replenishReserves() or provide liquidity during times of downturn or panic. It's up to the strategy to call replenishReserves() on the Branch Port so that the Branch Port can call withdraw() on the strategy. At a time when liquidity is likely to be scarce and losses in strategies high there's no economic incentive to replenish the Branch Port.

It is highly likely that a significant event will result in liquidity partially sitting in a Port Strategy while users are rushing for exits to withdraw their underlying tokens Branch Ports and not all users will be able to withdraw their funds.

Without the ability to set the minimum reserve ratio to match market conditions Port Strategies could even use the Branch Port as a source of liquidity and 'borrow on the way down' to the minimum reserve ratio. This is likely to lead to a loss of funds for users that had bridged their underlying assets for htokens.

Proof of Concept

Take a scenario like a hack, depeg of a stablecoin, vulnerability in Arbitrum, hack of Maia, or market panic and the underlying token is also a strategy token with a minimum reserve ratio of 3e3 or 3000 or roughly 30% of the underlying asset. This means that 70% is available to Port Strategies enabled to use this underlying asset.

A significant event has users wanting to exchange htokens that have been bridged to Root.
User's on Arbitrum callOut and bridge their htokens on Root for underlying assets in a destination chain (let's say ETH as a flight to safety).
The message is conveyed to the Branch Bridge Executor that calls clearToken or clearTokens in the Branch Bridge Agent.
Users that are fast to withdraw will receive their underlying tokens back as the call from the Branch Bridge agent to withdraw() on the Branch Port will not revert as there is enough underlying tokens to withdraw.
The Port Strategy has borrowed up to 70% of underlying tokens via manage() but has either sustained losses, has underlying funds locked in other markets and can't or won't withdraw them to replenishReserves() of the Branch Port.
The Branch Port will eventually get to a point where the balance of the underlying is less than the minimum reserve ratio required (3e3 or 3000) and Port Strategies will no longer be able to borrow from the Branch Port.
However with Port Strategies unwilling or unable to repay, and 70% of underlying assets potentially residing in Port Strategies not all users will receive their funds.
Even if the MaiaDAO acted quickly and the ratio of reserves to borrows was at 70%, Port Strategies would be able to remove liquidity down to the 30% or 3e3 as there's no way for MaiaDAO to react to market conditions and immediately set a higher minimum reserves ratio for the underlying asset. Port Strategies could borrow on the way down and the Branch Port could act as exit liquidity.

Tools Used

Vim

Recommended Mitigation Steps

MaiaDao should have the ability to set the minimum reserve ratios to a higher ratio at any time. This would allow the protocol to react to a serious event by increasing the reserves in phases, per underlying token per chain. Once liquidity returns to the Branch Port, Port Strategies would be able to withdraw again via manage() above the new minimum reserve ratio. This is a better strategy then just toggling Port Strategies off via togglePortStrategy() and freezing all ability to borrow. Once market conditions improve the protocol could reduce the minimum reserve ratio but require a minimum amount of time to pass before it is implemented.

Secondly there should be some way to reach in and remove liquidity from a Port Strategy. This might be underlying tokens that have not been invested but are sitting in the Port Strategy or even calling a function that removes funds from other markets and returns them to the Branch Port. This is a common pattern for strategies where some liquidity can be returned at lower cost but where investments need to be unwound a higher fee is paid. This would invert the current protocol design where Port Strategies replenish reserves when they want to.

Assessed type

Governance

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as low quality report

0xA5DF commented 1 year ago

In the event of a significant market issue like a vulnerability, hack or depeg user's will seek to bridge back their htokens for the original underlying asset creating a rush on liquidity in Branch Ports.

QA

alcueca commented 1 year ago

The functioning being described can be achieved by removing and re-adding with new settings. But what is being suggested can be seen as a great UX improvement.

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)