Lack of precision

[code423n4]

Handle

certora

Vulnerability details

Impact

In StabilityPool line 747 you multiply a sum of two parameters but since the second parameter is calculated through division it can cause lack of precision since the order of the operations is div()->mul() instead of mul()->div()

Tools Used

Manual code review

Recommended Mitigation Steps

dont divide secondPortion by SCALE_FACTOR, instead multiply the firstPortion by SCALE_FACTOR and after you multiply their sum by initialDeposit, divide the result by SCALE_FACTOR. like this:

original:

uint256 firstPortion = epochToScaleToSum[asset][snapshots.epoch][snapshots.scale].sub(
            S_Snapshot
        );        
        uint256 secondPortion = epochToScaleToSum[asset][snapshots.epoch][snapshots.scale.add(1)]
            .div(SCALE_FACTOR);

        uint256 assetGain = initialDeposit.mul(firstPortion.add(secondPortion)).div(P_Snapshot).div(
            DECIMAL_PRECISION
        );

changed:

uint256 firstPortion = epochToScaleToSum[asset][snapshots.epoch][snapshots.scale].sub(
            S_Snapshot
        ).mul(SCALE_FACTOR); 
        uint256 secondPortion = epochToScaleToSum[asset][snapshots.epoch][snapshots.scale.add(1)];

        uint256 assetGain = initialDeposit.mul(firstPortion.add(secondPortion)).div(SCALE_FACTOR).div(P_Snapshot).div(
            DECIMAL_PRECISION
        );

[kingyetifinance]

@LilYeti: This is intended behavior, and acts the same in Liquity. https://github.com/liquity/dev/blob/main/packages/contracts/contracts/StabilityPool.sol#L661-L678

[alcueca]

The sponsor acknowledged the issue, since it is not stated how the loss of precision is better than the alternative.