Ironsidesec - Wrong way of rounding when expectedAmounts calculation on withdrawal processing

[sherlock-admin2]

Ironsidesec

Medium

Wrong way of rounding when expectedAmounts calculation on withdrawal processing

Summary

When expectedAmounts are calculated, the denominator must be rounded up, but here already using rounded down denominator is used in a denominator which will inflate the end value.

Vulnerability Detail

1. ratiosX96Value in L580 below is calculated with mulDiv which will round down mostly due to volatile prices, and more volatile if underlying tokens > 1.
2. this ratiosX96Valuev is used to calculate coefficientX96 in denominator on L536 below. So using a rounded down value will inflate the `` value.
3. This inflated coefficientX96 is used as a numerator on L545 below calculating the expectedAmounts. Which will be inflated and rounding is in favor of a user. So he might get more than he deserves and also this will definitely revert when last withdrawer tries to withdraw his Lp, it might revert.

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/Vault.sol#L476-L534

2024-06-mellow/mellow-lrt/src/Vault.sol


557:     function calculateStack()
561:     {
... SNIP ...
577:         for (uint256 i = 0; i < tokens.length; i++) {
578:             uint256 priceX96 = priceOracle.priceX96(address(this), tokens[i]);
579:             s.totalValue += FullMath.mulDiv(amounts[i], priceX96, Q96); 
580:    >>>      s.ratiosX96Value += FullMath.mulDiv(s.ratiosX96[i], priceX96, Q96); 
581:             s.erc20Balances[i] = IERC20(tokens[i]).balanceOf(address(this));
582:         }
583:     }
584: 


525:     function analyzeRequest(
... SNIP ...
534:         uint256 value = FullMath.mulDiv(lpAmount, s.totalValue, s.totalSupply);
535:         value = FullMath.mulDiv(value, D9 - s.feeD9, D9);
536:   >>>   uint256 coefficientX96 = FullMath.mulDiv(value, Q96, s.ratiosX96Value);

... SNIP ...
541:         for (uint256 i = 0; i < length; i++) {
542:             uint256 ratiosX96 = s.ratiosX96[i];
543:             expectedAmounts[i] = ratiosX96 == 0
544:                 ? 0
545:    >>>          : FullMath.mulDiv(coefficientX96, ratiosX96, Q96);
546:             if (expectedAmounts[i] >= request.minAmounts[i]) continue;
547:             return (false, false, expectedAmounts);
548:         }
... SNIP ...
554:     }
555: 

Impact

Inflated coefficientX96 is used as numerator on L545 calculating the expectedAmounts. Which will be inflated and rounding is in favor of user. So he might get more than he deserves and also this will definitely revert when last withdrawer tries to withdraw his Lp, it might revert.

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/Vault.sol#L476-L534

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/Vault.sol#L530

Tool used

Manual Review

Recommendation

Use mulDivUp

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/Vault.sol#L530

 function calculateStack()
 public
 view
 returns (ProcessWithdrawalsStack memory s)
 {
... SNIP ...

 IPriceOracle priceOracle = IPriceOracle(configurator.priceOracle());
 for (uint256 i = 0; i < tokens.length; i++) {
 uint256 priceX96 = priceOracle.priceX96(address(this), tokens[i]);
 s.totalValue += FullMath.mulDiv(amounts[i], priceX96, Q96); 
-           s.ratiosX96Value += FullMath.mulDiv(s.ratiosX96[i], priceX96, Q96); 
+           s.ratiosX96Value += FullMath.mulDivUp(s.ratiosX96[i], priceX96, Q96); 
 s.erc20Balances[i] = IERC20(tokens[i]).balanceOf(address(this));
 }
 }

Duplicate of #61