Link to Issue: code-423n4/2023-03-asymmetry-findings#1078
While the "division before multiplication" issues described in M-01 have been mitigated in the proposed changeset, there are other cases which should be addressed too.
- In
SafEth::stake
the calculation ofpreDepositPrice
(now present in the functionapproxPrice()
) multipliesunderlyingValue
by 1e18, butunderlyingValue
is first divided by 1e18:
function approxPrice() public view returns (uint256) {
uint256 safEthTotalSupply = totalSupply();
uint256 underlyingValue = 0;
uint256 count = derivativeCount;
for (uint256 i = 0; i < count; i++) {
if (!derivatives[i].enabled) continue;
IDerivative derivative = derivatives[i].derivative;
underlyingValue +=
(derivative.ethPerDerivative() * derivative.balance()) /
1e18;
}
if (safEthTotalSupply == 0 || underlyingValue == 0) return 1e18;
return (1e18 * underlyingValue) / safEthTotalSupply;
}
- Also in
SafEth::stake
, something similar happens withtotalStakeValueEth
, as this is the sum ofderivativeReceivedEthValue
that is divided by 1e18, but thentotalStakeValueEth
is multiplied by 1e18 in the calculation ofmintAmount
:
uint256 totalStakeValueEth = 0; // total amount of derivatives staked by user in eth
for (uint256 i = 0; i < count; i++) {
if (!derivatives[i].enabled) continue;
uint256 weight = derivatives[i].weight;
if (weight == 0) continue;
IDerivative derivative = derivatives[i].derivative;
uint256 ethAmount = (msg.value * weight) / totalWeight;
if (ethAmount > 0) {
// This is slightly less than ethAmount because slippage
uint256 depositAmount = derivative.deposit{value: ethAmount}();
uint256 derivativeReceivedEthValue = (derivative
.ethPerDerivative() * depositAmount) / 1e18;
totalStakeValueEth += derivativeReceivedEthValue;
}
}
// mintedAmount represents a percentage of the total assets in the system
mintedAmount = (totalStakeValueEth * 1e18) / preDepositPrice;
- In
Reth::deposit
,rethPerEth
is calculated as(1e36) / ethPerDerivative()
but thenrethPerEth
is used in theminOut
calculation that ends dividing again by 1e36. The expression can be simplified to avoid any division before multiplication. The same applies toidealOut
:
uint256 rethPerEth = (1e36) / ethPerDerivative();
uint256 minOut = ((rethPerEth * msg.value) * (1e18 - maxSlippage)) /
1e36;
uint256 idealOut = (rethPerEth * msg.value) / 1e18;
Fix these other cases of "division before multiplication". In most of these, the expressions can be simplified as there is a division that is later multiplied by the same number.