sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

pks_ - Malicious token creator can change token decimals to make token price abnormally and cause contract asset stolen #49

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

pks_

high

Malicious token creator can change token decimals to make token price abnormally and cause contract asset stolen

Summary

IchiVaultOracle#getPrice use t0Decimal and t1Decimal to calculate totalReserve and token price. However, t0Decimal and t1Decimal can be manipulated by malicious token creators. So it's vulnerable when token decimals change and the token price may inflate or deflate, which will cause contract assets stolen.

Vulnerability Detail

    function getPrice(address pair) external override returns (uint256) {
        IUniswapV2Pair pool = IUniswapV2Pair(pair);
        uint256 totalSupply = pool.totalSupply();
        if (totalSupply == 0) return 0;

        address token0 = pool.token0();
        address token1 = pool.token1();

        (uint256 r0, uint256 r1, ) = pool.getReserves();
        uint256 px0 = base.getPrice(token0);
        uint256 px1 = base.getPrice(token1);
        uint256 t0Decimal = IERC20Metadata(token0).decimals();
        uint256 t1Decimal = IERC20Metadata(token1).decimals();
        uint256 sqrtK = BBMath.sqrt(
            r0 * r1 * 10 ** (36 - t0Decimal - t1Decimal)
        );

        return (2 * sqrtK * BBMath.sqrt(px0 * px1)) / totalSupply;
    }

and

    function getPrice(address token) external override returns (uint256) {
        IICHIVault vault = IICHIVault(token);
        uint256 totalSupply = vault.totalSupply();
        if (totalSupply == 0) return 0;

        address token0 = vault.token0();
        address token1 = vault.token1();

        /// Check price manipulations on Uni V3 pool by flashloan attack
        uint256 spotPrice = spotPrice0InToken1(vault);
        uint256 twapPrice = twapPrice0InToken1(vault);
        uint256 maxPriceDeviation = maxPriceDeviations[token0];
        if (!_isValidPrices(spotPrice, twapPrice, maxPriceDeviation))
            revert Errors.EXCEED_DEVIATION();

        /// Total reserve / total supply
        (uint256 r0, uint256 r1) = vault.getTotalAmounts();
        uint256 px0 = base.getPrice(address(token0));
        uint256 px1 = base.getPrice(address(token1));
        uint256 t0Decimal = IERC20Metadata(token0).decimals();
        uint256 t1Decimal = IERC20Metadata(token1).decimals();

        uint256 totalReserve = (r0 * px0) /
            10 ** t0Decimal +
            (r1 * px1) /
            10 ** t1Decimal;

        return (totalReserve * 10 ** vault.decimals()) / totalSupply;
    }

Impact

The contract assets may be stolen by malicious token creators.

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/oracle/IchiVaultOracle.sol#L124-L152 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/oracle/UniswapV2Oracle.sol#L40-L57

Tool used

vscode, Manual Review

Recommendation

Use cache mechanism to cache token decimals to prevent token decimal changed.

reference: https://github.com/sherlock-audit/2023-06-bond-judging/issues/90

sherlock-admin2 commented 1 year ago

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

shogoki commented:

Tokens are whitelisted

0xyPhilic commented:

invalid because it is irrelevant considering that only whitelisted tokens will be supported accross the protocol

darkart commented:

Invalid

Kral01 commented:

only whitelisted tokens are used in the protocol