code423n4 commented 1 year ago

Lines of code

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 https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L597 https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L606

Vulnerability details

Proof of Concept

In multiple places in the code, when doing calculations with both debt and price (of collateral) there is a multiplication by 1e18 - * 1 ether.

We have the following calculations:

uint minimumCollateral = debt * 1 ether / oracle.getPrice(address(collateral), collateralFactorBps) * 10000 / collateralFactorBps;

uint price = oracle.getPrice(address(collateral), collateralFactorBps);
uint liquidatorReward = repaidDebt * 1 ether / price;

uint liquidationFee = repaidDebt * 1 ether / price * liquidationFeeBps / 10000;

Let’s say the collateral used was a non-18 decimals token, for example USDT (6 decimals). Looking at the getPrice() method from Oracle.sol we see the following code:

uint price = feeds[token].feed.latestAnswer();
uint8 feedDecimals = feeds[token].feed.decimals();
uint8 tokenDecimals = feeds[token].tokenDecimals;
uint8 decimals = 36 - feedDecimals - tokenDecimals;
uint normalizedPrice = price * (10 ** decimals);

The “USDT/USD” price feed in Chainlink has 18 decimals. Following the math we will see that normalizedPrice will have 12 more decimals than USDT’s price feed, 30 in total.

Now if we go back again to, for example, this math

uint liquidationFee = repaidDebt * 1 ether / price * liquidationFeeBps / 10000;

The debt always has 18 decimals, because the Dola token has 18 decimals. But if we multiply it by 1e18 or 1 ether now it has 36 decimals, even though the price has not been “normalized” to have 18 decimals and it has only 12. This will result in the result from repaidDebt * 1 ether / price having 24 decimals instead of 18, and doing * liquidationFeeBps / 10000; after it will result in a much bigger liqudationFee. This results in 2 scenarios:

If escrow has enough balance, it will pay the huge liquidation fee to the protocol, which is a loss for the protocol users.
If escrow does not have enough balance in this case it won’t pay any liquidaton fee, but since we do the same math for liquidatorReward, there we have the same case and if escrow does not have enough balance it will result in a DoS, which means all debts won’t be liquidateable.

The same thing applies for the other places that we have the debt * 1 ether math.

Impact

The impact of this is a loss of value for users or a DoS on core protocol functionality. It can only happen if a non-18 decimal token is used, but this can easily be the case if USDT is used as collateral, hence the High severity.

Recommendation

Enforce only 18 decimals tokens to be used in a Market or just properly get the price to always be with 18 decimals in Oracle.sol's getPrice() & viewPrice()