sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

evilakela - IronBank::_getExchangeRate doesn't account for market decimals #487

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

evilakela

high

IronBank::_getExchangeRate doesn't account for market decimals

Summary

IronBank::_getExchangeRate doesn't account for market decimals resulted in incorrect rates.

Vulnerability Detail

function _getExchangeRate(DataTypes.Market storage m) internal view returns (uint256) {
    uint256 totalSupplyPlusReserves = m.totalSupply + m.totalReserves;

    if (totalSupplyPlusReserves == 0) {
        return m.config.initialExchangeRate;
    }

    return ((m.totalCash + m.totalBorrow) * 1e18) / totalSupplyPlusReserves; 
}

m.totalCash & m.totalBorrow has market decimal precision, but _getExchangeRate uses 1e18 for precision scaling. This lead to incorrect exchange rates for markets with decimals != 18.

Impact

Incorrect calculation of exchange rates break protocol

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L805-L811

Tool used

Manual Review

Recommendation

I suggest use m.config.initialExchangeRate as it's set to correct 10 ** market.decimals() precision:

function _getExchangeRate(DataTypes.Market storage m) internal view returns (uint256) {
    uint256 totalSupplyPlusReserves = m.totalSupply + m.totalReserves;

    if (totalSupplyPlusReserves == 0) {
        return m.config.initialExchangeRate;
    }

    return ((m.totalCash + m.totalBorrow) * m.config.initialExchangeRate) / totalSupplyPlusReserves; 
}

Note

Same issues with _isLiquidatable and _getAccountLiquidity: 1e18 decimal presicion used for all markets. Solution would be also using m.config.initialExchangeRate instead of 1e18. I don't think it's different vulnerability as underlaing ussue the same.