Potential DOS to attempts in getting the LSTs out of the protocol

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BridgeRouterFacet.sol#L101-L123

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BridgeRouterFacet.sol#L101-L123

    function withdraw(address bridge, uint88 dethAmount) external nonReentrant {
        if (dethAmount == 0) revert Errors.ParameterIsZero();

        (uint256 vault, uint256 bridgePointer) = _getVault(bridge);

        uint88 fee;
        if (vault == VAULT.ONE) {
            uint88 dethAssessable = vault.assessDeth(bridgePointer, dethAmount, rethBridge, stethBridge);
            if (dethAssessable > 0) {
                uint256 withdrawalFeePct = LibBridgeRouter.withdrawalFeePct(bridgePointer, rethBridge, stethBridge);
                if (withdrawalFeePct > 0) {
                    fee = dethAssessable.mulU88(withdrawalFeePct);
                    dethAmount -= fee;
                    s.vaultUser[vault][address(this)].ethEscrowed += fee;
                }
            }
        }

        uint88 ethAmount = _ethConversion(vault, dethAmount);
        vault.removeDeth(dethAmount, fee);
        IBridge(bridge).withdraw(msg.sender, ethAmount);
        emit Events.Withdraw(bridge, msg.sender, dethAmount, fee);
    }

This function is used in order to withdraw LST out of the protocol, it inlcudes an important logic of deducting the the withdrwal fee percentage, i.e https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L111-L141

    function withdrawalFeePct(uint256 bridgePointer, address rethBridge, address stethBridge) internal view returns (uint256 fee) {
        IBridge bridgeReth = IBridge(rethBridge);
        IBridge bridgeSteth = IBridge(stethBridge);

        // Calculate rETH market premium/discount (factor)
        uint256 unitRethTWAP = OracleLibrary.estimateTWAP(1 ether, 30 minutes, VAULT.RETH_WETH, VAULT.RETH, C.WETH);
        uint256 unitRethOracle = bridgeReth.getUnitDethValue();
        uint256 factorReth = unitRethTWAP.div(unitRethOracle);
        // Calculate stETH market premium/discount (factor)
        uint256 unitWstethTWAP = OracleLibrary.estimateTWAP(1 ether, 30 minutes, VAULT.WSTETH_WETH, VAULT.WSTETH, C.WETH);
        uint256 unitWstethOracle = bridgeSteth.getUnitDethValue();
        uint256 factorSteth = unitWstethTWAP.div(unitWstethOracle);
        if (factorReth > factorSteth) {
            // rETH market premium relative to stETH
            if (bridgePointer == VAULT.BRIDGE_RETH) {
                // Only charge fee if withdrawing rETH
                return factorReth.div(factorSteth) - 1 ether;
            }
        } else if (factorSteth > factorReth) {
            // stETH market premium relative to rETH
            if (bridgePointer == VAULT.BRIDGE_STETH) {
                // Only charge fee if withdrawing stETH
                return factorSteth.div(factorReth) - 1 ether;
            }
        } else {
            // Withdrawing less premium LST or premiums are equivalent
            return 0;
        }
    }

So this function is used to bridge the fees so as to prevent free arbitrage, fee charged is the premium/discount differential, keep in mind that this function is also called whenever BridgeRouterFacet.withdraw() is called, case here however is that it hardcodes the call to always route it via the UniswapOracleLibrary's estimateTwap() to get the current price , now iin estimateTwap() there is a call to pool.observe() , i.e https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/UniswapOracleLibrary.sol#L54-L60

        uint32[] memory secondsAgos = new uint32[](2);
        secondsAgos[0] = secondsAgo;
        secondsAgos[1] = 0
        (int56[] memory tickCumulatives,) = IUniswapV3Pool(pool).observe(secondsAgos);

Case is that the call to pool.observe() could fail for whatever reason, say lack of enough history and so on and as such cause the whole attempt at withdrawing the LSTs to be broken.

Impact

In the worst case this leads to DOS on all attempts of withdrawing the LSTs out of the protocol due to the heavy indirect dependence on the pool.observe()

Recommended Mitigation Steps

Reconsider the idea of hardcoding the call to estimateTwap() via withdrawalFeePct() it should instead be done in a try-catch and then if the call to estimateTwap() fails for whatever reason in withdrawalFeePct() then another provider should be used to get the price for the fee's calculation.

Assessed type

Uniswap

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Devoid of numerical examples to support your claim.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof