sherlock-audit/2023-03-notional-judging

chaduke - convertFromStorage() fails to use rounding-up when converting a negative storedCashBalance into signedPrimeSupplyValue.

Opened this issue · 0 comments

chaduke

medium

convertFromStorage() fails to use rounding-up when converting a negative storedCashBalance into signedPrimeSupplyValue.

Summary

convertFromStorage() fails to use rounding-up when converting a negative storedCashBalance into signedPrimeSupplyValue.

Vulnerability Detail

convertFromStorage() is used to convert storedCashBalance into signedPrimeSupplyValue. When storedCashBalance is negative, it represents a debt - positive prime cash owed.

https://github.com/sherlock-audit/2023-03-notional/blob/main/contracts-v2/contracts/internal/pCash/PrimeRateLib.sol#L60-L74

Unfortunately, when converting a negative storedCashBalance into signedPrimeSupplyValue, the following division will apply a rounding-down (near zero) mode, leading to a user to owe less than it is supposed to be.

return storedCashBalance.mul(pr.debtFactor).div(pr.supplyFactor);

This is not acceptable. Typically, rounding should be in favor of the protocol, not in favor of the user to prevent draining of the protocol and losing funds of the protocol.

The following POC shows a rounding-down will happen for a negative value division. The result of the following test is -3.

function testMod() public {
         
         int256 result = -14;
         result = result / 4;
         console2.logInt(result);
    }

Impact

convertFromStorage() fails to use rounding-up when converting a negative storedCashBalance into signedPrimeSupplyValue. The protocol is losing some dusts amount, but it can be accumulative or a vulnerability that can be exploited.

Code Snippet

Tool used

VSCode

Manual Review

Recommendation

Use rounding-up instead.

function convertFromStorage(
        PrimeRate memory pr,
        int256 storedCashBalance
    ) internal pure returns (int256 signedPrimeSupplyValue) {
        if (storedCashBalance >= 0) {
            return storedCashBalance;
        } else {
            // Convert negative stored cash balance to signed prime supply value
            // signedPrimeSupply = (negativePrimeDebt * debtFactor) / supplyFactor

            // cashBalance is stored as int88, debt factor is uint80 * uint80 so there
            // is no chance of phantom overflow (88 + 80 + 80 = 248) on mul
-           return storedCashBalance.mul(pr.debtFactor).div(pr.supplyFactor);
+         return (storedCashBalance.mul(pr.debtFactor).sub(pr.supplyFactor-1)).div(pr.supplyFactor);
        }
    }