Ethos Reserve contest - KingNFT's results

Lines of code

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

Vulnerability details

Impact

The _rebalance() of ActivePool contract may revert due to arithmetic underflow, results in the majority of functionality of Ethos-Core becoming temporarily unavailable.

Proof of Concept

The issue arises on L251 of ActivePool.sol, as the convert between Assets and Shares are rounded down, so the vars.sharesToAssets has a high chance to be less than vars.currentAllocated in the period that a new deposit to vault has been done but no enough new yield gain is generated to compensate for the rounding down loss.

File: Ethos-Core\contracts\ActivePool.sol
239:     function _rebalance(address _collateral, uint256 _amountLeavingPool) internal {
240:         LocalVariables_rebalance memory vars;
241: 
242:         // how much has been allocated as per our internal records?
243:         vars.currentAllocated = yieldingAmount[_collateral];
244:         
245:         // what is the present value of our shares?
246:         vars.yieldGenerator = IERC4626(yieldGenerator[_collateral]);
247:         vars.ownedShares = vars.yieldGenerator.balanceOf(address(this));
248:         vars.sharesToAssets = vars.yieldGenerator.convertToAssets(vars.ownedShares);
...
251:         vars.profit = vars.sharesToAssets.sub(vars.currentAllocated); // @audit might revert
252:         if (vars.profit < yieldClaimThreshold[_collateral]) {
253:             vars.profit = 0;
254:         }
...
309:     }

Let's illustrate with an example Given initial state

collateral = WETH
activePoolShares = 500
activePoolYieldingAmount = 555
totalSharesOfValut = 1000
totalAssetsOfValut = 1110

this is an idea state with exactly half shares half assets, no convert rounding down error, but any change on it will have a high chance to introduce rounding error. We can see it in the following steps.

The active pool deposits another 100 wei to the vault, then

activePoolYieldingAmount = 555 + 100 = 655

and according to L334 of _deposit() of ReaperVaultV2.sol

File: Ethos-Vault\contracts\ReaperVaultV2.sol
319:     function _deposit(uint256 _amount, address _receiver) internal nonReentrant returns (uint256 shares) {
...
331:         if (totalSupply() == 0) {
332:             shares = _amount;
333:         } else {
334:             shares = (_amount * totalSupply()) / freeFunds; // use "freeFunds" instead of "pool"
335:         }
336:       _mint(_receiver, shares);
...
338:     }

the active pool will receive

addedAtcivePoolShare = 100 * 1000 / 1110 = 90 // rounding down

and

totalAssetsOfValut = 1110 + 100 = 1210
totalSharesOfValut = 1000 + 90 = 1090
activePoolShares = 500 + 90 = 590

From this time point till the vault yields enough new gain, vars.sharesToAssets.sub(vars.currentAllocated) triggers revert

vars.sharesToAssets = 590 * 1210 / 1090 = 654 // rounding down
vars.currentAllocated = activePoolYieldingAmount  = 655
vars.sharesToAssets.sub(vars.currentAllocated) = 654 - 655 // trigger revert

As the _rebalance() will be called on any collateral change of ActivePool, this issue will cause the majority of operations of Ethos-Core becoming temporarily unavailable.

Tools Used

Manually review

Recommended Mitigation Steps

Check if vars.sharesToAssets is larger than vars.currentAllocated before sub.