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.
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);
}
}