Ethos Reserve contest - 0xBeirao's results

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/StabilityPool.sol#L526-L538

Vulnerability details

Description

updateRewardSum function call _computeRewardsPerUnitStaked with _debtToOffset set to 0. Meaning that the assignment L531 will revert if lastLUSDLossError_Offset != 0 (which is likely the case) because we try to assign a negative value to an uint.

Impact

_rebalance() will be definitely DOS if the profit is greater than the yieldClainThreshold ⇒ vars.profit != 0.

Because they call _rebalance() all these functions will be DOS :

In BorrowerOperations 100% DOS - openTrove - closeTrove - _adjustTrove - addColl, withdrawColl - withdrawLUSD, repayLUSD In TroveManager 80% DOS - liquidateTroves - batchLiquidateTroves - redeemCloseTrove

POC

Context : the vault has compound enough profit to withdraw. (here)

Alice initiates a trove liquidation. offset() in StabilityPool is called to cancels out the trove debt against the LUSD contained in the Stability Pool.

A floor division errors occur so now lastLUSDLossError_Offset is not null.

Now, every time _rebalance() is called the transaction will revert.

Mitigation

In StabilityPool.sol#L504-L544, just skip the floor division errors calculation if _debtToOffset == 0

if(_debtToOffset != 0){
	[StabilityPool.sol#L526-L538](https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/StabilityPool.sol#L526-L538)
}

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/LQTY/LQTYStaking.sol#L181

Vulnerability details

Description

Due to decimal difference not taken into account, the increaseF_Collateral() function from LQTYStaking contract will not increase the reward per unit staked.

Impact

If WBTC is used in collateral ; when collecting the 0.5% fee (here) or when crediting the vault yield (here) and under this conditions :

$collFeePerLQTYStaked = \frac{collWBTC*1^{18}}{totalLQTYStaked} <1 ⇔ collWBTC < \frac{totalLQTYStaked}{1^{18}}$

The LQTYStaking contract will not be credited, making these tokens to be lost forever and the LQTY stakers not receiving their rewards.

Because WBTC has 8 decimals and LQTY has 18 decimals, it’s easy to find values that satisfy this condition.

POC

We call increaseF_Collateral(address _collateral, uint collFee) because we want to increase the reward per unit staked in the LQTYStakingcontract.

_collFee = 0.001 WBTC = 1e5 wsatoshi

totalLQTYStaked = 1_000_000 LQTY = 1e24 wei

With these values the bug condition is verified :

Mitigation

One solution could be to keep the rounding error in a variable and credit the reward to stakers when it can be apply. Similarly to the StabilityPool contract with the lastCollateralError_Offset variable. StabilityPool.sol#L524-L541

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/ActivePool.sol#L251

Vulnerability details

Description

Ethos core contracts are not designed to deal with losses. This is because vaults invest in strategies that are not designed to lose money.

Impact/POC

Globally, this system doesn't handle losses, but let's take an example to show why this is critical:

Let's say a vault strategy is hacked and 10% of the total collateral disappears.

This assignment will now revert every time _rebalance() is called : ActivePool.sol#L251

vars.profit = vars.sharesToAssets.sub(vars.currentAllocated);

Now sharesToAssets < currentAllocated. Because we try to assign a negative value to an uint , _rebalance will now always revert making the system DOS :

In BorrowerOperations 100% DOS

• openTrove
• closeTrove
• _adjustTrove
• addColl, withdrawColl
• withdrawLUSD, repayLUSD In TroveManager 80% DOS
• liquidateTroves
• batchLiquidateTroves
• redeemCloseTrove

This will probably trigger fear in users and they will start selling their LUSD on the secondary markets causing a depeg. Probably nobody will want to buy back LUSD since the system is DOS users are no longer incentivises to maintain the peg.

Note that a hack or bug in a strategy that results in a loss of funds of just a few basis points can trigger this massive DOS.

With 2 examples we will figure out why this is really concerning

1st scenario (the repairable one) :

Context : TVL = 1 000 000 ; lossPercentage = 0.01% ⇒ loss = 10 000$

Here 10k have been hacked from a strategy. The loss is small but the protocol DOS anyway. The Ethos team can fix the protocol by manually sending 10k$ of the underlying collateral to the vault that has been hacked in order artificially increase the share price.

Impact : protocol DOS during 1 day and Ethos loss 10 000$

2nd scenario :

Context : TVL = 1B ; lossPercentage = 10% ⇒ loss = 100M$

Now the team will have to send 100M to the vault to repair the protocol. If they never find the money the other 900M$ will be stuck in the protocol forever. Making users lose 100% of their investment instead of only 10%.

Mitigation

This scenario needs to be handled properly.

A safer system would be to spread the loss over all investors.

If we take the example above, everyone loses 10%. The TCR will fall, causing some troves to be liquidated. If the loss is greater than 10%, some troves may be underwater. Liquidity providers of stability pool will absorb the loss, but at least the integrity of the system is maintained and the LUSD remains pegged.

Even though the strategies invested in are the safest in the industry, I think it's worth implementing a loss-handling mechanism to ensure that the protocol remains robust over time.