c4-bot-7 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L131-L143 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L113-L141

Vulnerability details

Summary

There are checks missing on prices returned by the estimateTWAP function

Vulnerability Details

Essential price checks carried out in LibOracle::twapCircuitBreaker() are not similarly carried out in LibBridgeRouter::withdrawalFeePct(). twapCircuitBreaker() reverts if the WETH/USDC pool returns 0 or if the pool has less than 100 Ether which would make it unreliable and manipulatable. This is more important for rEth/wEth & stETH/wETH whose pools already have far less liquidity than WETH/USDC and are likelier to break this threshold.

    function twapCircuitBreaker() private view returns (uint256 twapPriceInEth) {
        // Check valid price
        uint256 twapPrice = IDiamond(payable(address(this))).estimateWETHInUSDC(C.UNISWAP_WETH_BASE_AMT, 30 minutes);
>>>     if (twapPrice == 0) revert Errors.InvalidTwapPrice();

        // Check valid liquidity
        IERC20 weth = IERC20(C.WETH);
        uint256 wethBal = weth.balanceOf(C.USDC_WETH);
>>>     if (wethBal < 100 ether) revert Errors.InsufficientEthInLiquidityPool();

        // SOME CODE
    }

Impact

If 0 values are returned this will results in users paying no fees and the protocol losing money. Low liquidity pools are inherently more volatile and can be manipulated by malicious users.

Tools Used

Manual Review Foundry Testing

Recommendations

withdrawalFeePct() should implement the same checks to ensure that returned prices are accurate.

Assessed type

Oracle

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

raymondfam commented 5 months ago

Inadequate elaboration given. QA at best.

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

c4-judge commented 4 months ago

hansfriese changed the severity to QA (Quality Assurance)