Asymmetry contest - mojito_auditor's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L88

Vulnerability details

Impact

In the function SafEth.stake(), it does a for loop through derivative[] to deposit on each of them based on weights[]. However, it rounds down when calculate the ethAmount to deposit.

for (uint i = 0; i < derivativeCount; i++) {
    uint256 weight = weights[i];
    IDerivative derivative = derivatives[i];
    if (weight == 0) continue;
    
    // @audit rounding, lock few weis
    uint256 ethAmount = (msg.value * weight) / totalWeight; 

    // This is slightly less than ethAmount because slippage
    uint256 depositAmount = derivative.deposit{value: ethAmount}();
    uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
        depositAmount
    ) * depositAmount) / 10 ** 18;
    totalStakeValueEth += derivativeReceivedEthValue;
}

As a result, a few wei could be locked in the SafEth contract while it should be deposited to any derivative or return to users.

Proof of Concept

Consider the scenario

1. There are 3 derivatives with weights = [31, 31, 34], totalWeights = 97
2. Bob stake msg.value = 100. So now when calculate

ethAmount_1 = 100 * 31 / 97 = 31
ethAmount_2 = 100 * 31 / 97 = 31
ethAmount_3 = 100 * 34 / 97 = 35
sum = 31 + 31 + 35 = 97

As we can see, total deposited amount is 97 while Bob sent in 100. There are 3 weis is locked in contract forever.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider depositing all amount left to the last derivative or transfer it back users.