There is no consideration about difference of decimals of JUSD and collaterals in JUSDBank.

Summary

JUSDBank uses several collaterals. But there is no consideration about the case that decimals of collaterals are different each other and the case that the decimals of collaterals and JUSD are different. In such cases, there could be large error in the calculation.

Vulnerability Detail

JUSDBank records the amount of collaterals without converting decimals. For example, JUSDBank.sol#_deposit function is the following.

    function _deposit(
        Types.ReserveInfo storage reserve,
        Types.UserInfo storage user,
        uint256 amount,
        address collateral,
        address to,
        address from
    )
        internal
    {
        require(reserve.isDepositAllowed, Errors.RESERVE_NOT_ALLOW_DEPOSIT);
        require(amount != 0, Errors.DEPOSIT_AMOUNT_IS_ZERO);
254:    IERC20(collateral).safeTransferFrom(from, address(this), amount);
        _addCollateralIfNotExists(user, collateral);
256:    user.depositBalance[collateral] += amount;
        reserve.totalDepositAmount += amount;
        .........
    }

As can be seen, L256 records the amount of collateral transferred in L254 to depositBalance without converting decimal. JUSDBank also records the amount of borrowed JUSD without converting decimal. For example, JUSDBank.sol#_borrow function is the following.

    function _borrow(
        Types.UserInfo storage user,
        bool isDepositToJOJO,
        address to,
        uint256 tAmount,
        address from
    )
        internal
    {
275:    uint256 t0Amount = tAmount.decimalRemainder(tRate) ? tAmount.decimalDiv(tRate) : tAmount.decimalDiv(tRate) + 1;
276:    user.t0BorrowBalance += t0Amount;
        t0TotalBorrowAmount += t0Amount;
        if (isDepositToJOJO) {
            IERC20(JUSD).approve(address(JOJODealer), tAmount);
            IDealer(JOJODealer).deposit(0, tAmount, to);
        } else {
282:        IERC20(JUSD).safeTransfer(to, tAmount);
        }
        require(
            user.t0BorrowBalance.decimalMul(tRate) <= maxPerAccountBorrowAmount,
            Errors.EXCEED_THE_MAX_BORROW_AMOUNT_PER_ACCOUNT
        );
        require(
            t0TotalBorrowAmount.decimalMul(tRate) <= maxTotalBorrowAmount, Errors.EXCEED_THE_MAX_BORROW_AMOUNT_TOTAL
        );
        emit Borrow(from, to, tAmount, isDepositToJOJO);
    }

As can be seen L276 records the amount of JUSD transferred in L282 to toBorrowBalance without converting decimal, where decimal of tRate in L275 is 18. JUSDView.sol#_isAccountSafe function estimates the safety of account using the amount of JUSD toBorrowBalance and the amount of collateral depositBalance which are recorded as above.

    function _getMintAmount(
        Types.ReserveInfo memory reserve,
        uint256 amount,
        uint256 rate
    )
        internal
        view
        returns (uint256)
    {
84:     uint256 depositAmount = IPriceSource(reserve.oracle).getAssetPrice().decimalMul(amount).decimalMul(rate);
        if (depositAmount >= reserve.maxColBorrowPerAccount) {
            depositAmount = reserve.maxColBorrowPerAccount;
        }
        return depositAmount;
    }

    function _isAccountSafe(Types.UserInfo storage user, uint256 tRate) internal view returns (bool) {
92:     return user.t0BorrowBalance.decimalMul(tRate) <= _maxMintAmount(user);
    }

    function _maxMintAmount(Types.UserInfo storage user) internal view returns (uint256) {
        address[] memory collaterals = user.collateralList;
        uint256 maxMintAmount;
        for (uint256 i; i < collaterals.length; i = i + 1) {
            address collateral = collaterals[i];
            Types.ReserveInfo memory reserve = reserveInfo[collateral];
            if (!reserve.isBorrowAllowed) {
                continue;
            }
            uint256 colMintAmount =
105:            _getMintAmount(reserve, user.depositBalance[collateral], reserve.initialMortgageRate);
106:        maxMintAmount += colMintAmount;
        }
        return maxMintAmount;
    }

In L84, amount is user.depositBalance[collateral] of L105 and the decimals of getAssetPrice() and rate are 18. Thus, the decimals of depositAmount in L84, colAmount in L05 and maxMintAmount in L106 are equal to the decimal of collateral.

But the decimals of collaterals are different each other. For instance, from the test code, we can see that the decimal of BTC is 8, decimal of eth is 18 and deciaml of JUSD is 6, where BTC and eth are used as collaterals. Thus, it turns out to be that L106 adds collaterals of different decimals and L92 compares values of different decimals.

Impact

This issue causes serious damage to the JUSDBank.

Code Snippet

https://github.com/sherlock-audit/2023-12-jojo-exchange-update/blob/main/smart-contract-EVM/src/JUSDBank.sol#L256 https://github.com/sherlock-audit/2023-12-jojo-exchange-update/blob/main/smart-contract-EVM/src/JUSDView.sol#L106

Tool used

Manual Review

Recommendation

The decimals of collaterals and JUSD should be converted to 18 when recording the amount of them.

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { no specific damage mentioned by watson like precision lost or anything}

nevillehuang commented 9 months ago

Invalid, for different collateral decimals, their prices also have different decimals. For example: eth decimal is 18 so that the price is based on 1e6, btc decimal is 18 so that the price is based on 1e16 to insurance colMintAmount is based on 1e6 .

uint256 colMintAmount = _getMintAmount(reserve, user.depositBalance[collateral], reserve.initialMortgageRate); maxMintAmount += colMintAmount;

sherlock-audit / 2023-12-jojo-exchange-update-judging

dany.armstrong90 - There is no consideration about difference of decimals of JUSD and collaterals in JUSDBank. #84