code-423n4 / 2022-10-inverse-findings

0 stars 0 forks source link

Divide before multiply #598

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L360

Vulnerability details

division before multiply

Impact

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision.

Proof of Concept

In general, this is a problem due to precision.

In this case, it also affects balance of the protocol, what makes me suggest high severity, as users can withdraw more than expected as minimumCollateral would get rounded down.

The amount of collateral needed in minimumCollateral to be less than expected, making the user able to withdraw more than expected in getWithdrawalLimitInternal and getWithdrawalLimit

        minimumCollateral = debt * 1000000000000000000 / oracle.viewPrice(address(collateral),collateralFactorBps) * 10000 / collateralFactorBps

https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L360 https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L377

Also, this would affect fees of the protocol in liquidate() as liquidationFee can be truncated, making the amount collected less than expected.

        liquidationFee = repaidDebt * 1000000000000000000 / price * liquidationFeeBps / 10000

https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L606

The replenisherReward, the liquidatorReward to be less than expected affecting reward.

        replenishmentCost = amount * dbr.replenishmentPriceBps() / 10000
    replenisherReward = replenishmentCost * replenishmentIncentiveBps / 10000

https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L563-L564

    liquidatorReward = repaidDebt * 1000000000000000000 / price
    liquidatorReward += liquidatorReward * liquidationIncentiveBps / 10000

https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L597-L598

Recommended Mitigation Steps

Reorder the operations. For more info: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

neumoxx commented 2 years ago

I don't see this as an issue (let alone a High one), it's just calculating percentages.

0xean commented 2 years ago

warden fails to show impact of rounding error.

c4-judge commented 2 years ago

0xean marked the issue as unsatisfactory: Insufficient quality