code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

Denial Of Service(Dos) due to ChainID Overflow Vulnerability #442

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/factories/BranchBridgeAgentFactory.sol#L20-L24 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/factories/RootBridgeAgentFactory.sol#L12-L13

Vulnerability details

Impact

Using uint16 to store chain IDs in smart contracts can have several significant impacts:

  1. Deployment Overflow: Smart contracts deployed on networks with chain IDs exceeding the uint16 maximum value (65535) will experience overflow issues during deployment, rendering the contract unusable on such networks. Example: Harmony Mainnet (1666600000) and Aurora Mainnet(1313161554)

  2. Testing Limitations: Users intending to test these contracts on testnets with higher chain IDs (e.g., Arbitrum Goerli, Sepolia Testnet, Polygon Mumbai Testnet) will face similar overflow issues, hindering effective testing and development.

  3. Future Compatibility: As new Layer 2 (L2) networks are introduced, there is a growing likelihood that they may have chain IDs beyond the uint16 limit. Contracts not designed to accommodate larger chain IDs will not be compatible with these networks.

Description

The issue primarily stems from the use of uint16 data types to store chain IDs in smart contracts. In Solidity, uint16 can only hold values from 0 to 65535. When a contract is deployed, it initializes its uint16 chain ID variables using constructor parameters. If the provided chain IDs exceed 65535, an overflow occurs, leading to unexpected behavior and contract inoperability on networks with higher chain IDs.

Additionally, contracts designed with uint16 chain ID variables are limited in their ability to adapt to new networks and Layer 2 solutions that may have chain IDs exceeding the 65535 threshold. This lack of adaptability restricts the contract's utility and potential use cases.

Proof of Concept

// SPDX-License-Identifier: BUSL-1.1

pragma solidity ^0.8.16;

contract Endpoint {

    uint16 public immutable chainId;
    //Test inputs
    //Harmony Mainnet (1666600000) and Aurora Mainnet(1313161554)
    //Arbitrum Goerli(421613), Sepolia Testnet (11155111) , Polygon Mumbai Testnet (80001)

    constructor(uint16 _chainId) {
        chainId = _chainId;
    }
}

Tools Used

Manual

Recommended Mitigation Steps

To enhance the compatibility of smart contracts across various blockchain networks, especially those with higher chain IDs replace uint16 with a larger data type, such as uint256, to store chain IDs. This is already being done in the other parts of the code (1, 2)

Assessed type

DoS

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as low quality report

0xA5DF commented 1 year ago

Smart contracts deployed on networks with chain IDs exceeding the uint16 maximum value (65535)

I need a concrete example

Testing Limitations Future Compatibility

Not in scope

alcueca commented 1 year ago

https://chainlist.org/

Valid QA.

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

alcueca marked the issue as grade-a