Decimals of LendingPool don't take into account the offset introduced by VIRTUAL_SHARES

MediumINIT Capital·Code4rena · Money market · 15th December, 2023CodebaseM-02

The impact of this finding is more on the marketing/data fetching side, on exchanges it would appear that the shares are worth less VIRTUAL_SHARES than the underlying token. Given that it would influence the perception of the value of the shares token, medium severity seems appropriate.

Proof of Concept

The Openzeppelin implementation includes the decimals offset (log10(VIRTUAL_SHARES) in LendingPool) in the decimals() function. However, INIT only places the decimals of the underlying.

A POC was built, add it to TestLendingPool.sol:

function test_POC_WrongDecimals() public {
    uint256 _wbtcAmount = 3e8; // 3 BTC
    address _user = makeAddr("user");
    _mintPool(_user, WBTC, _wbtcAmount);
    uint256 _wbtcDecimals = 1e8;
    uint256 VIRTUAL_SHARES = 1e8;
    uint256 _poolDecimals = 10**lendingPools[WBTC].decimals();
    uint256 _userBalance = lendingPools[WBTC].balanceOf(_user);
    assertEq(_userBalance/_poolDecimals, _wbtcAmount/_wbtcDecimals*VIRTUAL_SHARES);
    assertEq(_userBalance/_poolDecimals, 3e8);
    assertEq(_userBalance, _wbtcAmount*VIRTUAL_SHARES);
}

Tools Used

Vscode, Foundry

Recommended Mitigation Steps

Include the virtual shares decimals in the decimals() function:

uint private constant VIRTUAL_SHARES = 8;
...
function decimals() public view override returns (uint8) {
    return IERC20Metadata(underlyingToken).decimals() + VIRTUAL_SHARES;
}
...
function _toShares(uint _amt, uint _totalAssets, uint _totalShares) internal pure returns (uint shares) {
    return _amt.mulDiv(_totalShares + 10**VIRTUAL_SHARES, _totalAssets + VIRTUAL_ASSETS);
}
...
function _toAmt(uint _shares, uint _totalAssets, uint _totalShares) internal pure returns (uint amt) {
    return _shares.mulDiv(_totalAssets + VIRTUAL_ASSETS, _totalShares + 10**VIRTUAL_SHARES);
}