Lines of code
<https://github.com/pooltogether/ERC5164/blob/5647bd84f2a6d1a37f41394874d567e45a97bf48/src/ethereum-optimism/EthereumToOptimismRelayer.sol#L49-L78&gt;
<https://github.com/pooltogether/ERC5164/blob/5647bd84f2a6d1a37f41394874d567e45a97bf48/src/ethereum-polygon/EthereumToPolygonRelayer.sol#L45-L65&gt;
<https://github.com/pooltogether/ERC5164/blob/5647bd84f2a6d1a37f41394874d567e45a97bf48/src/ethereum-arbitrum/EthereumToArbitrumExecutor.sol#L26&gt;
<https://github.com/pooltogether/ERC5164/blob/5647bd84f2a6d1a37f41394874d567e45a97bf48/src/ethereum-optimism/EthereumToOptimismExecutor.sol#L29&gt;
<https://github.com/pooltogether/ERC5164/blob/5647bd84f2a6d1a37f41394874d567e45a97bf48/src/ethereum-polygon/EthereumToPolygonExecutor.sol#L31&gt;

Vulnerability details

Impact

Denial of service on relayCalls() functions when the nonce variable reach type(uint256).max

Proof of Concept

When the smart contracts start to be used, the variable in storage nonce will start to increment by 1, and since the nonce variable cannot be decremented (or changed), because every transaction must have a unique one (nonce) in order to prevent replay attacks. Therefore all the relayCalls() functions will generate a DoS in the future.

Take in mind that groupe of attackers could recursively call relayCalls() func in attempt to make this scenario much sooner (Sometimes, an attacker’s goal is to block a specific contract with no specific reason (for fun I guess)).

Tools Used

Manual review

Recommended Mitigation Steps

Use a mapping to map every user nonces separately :

mapping(address => uint256) public nonces;

function relayCalls(CallLib.Call[] calldata _calls, uint256 _gasLimit)
    external
    payable
    returns (uint256)
  {
    uint256 _maxGasLimit = maxGasLimit;

    if (_gasLimit > _maxGasLimit) {
      revert GasLimitTooHigh(_gasLimit, _maxGasLimit);
    }

    userNonces[msg.sender]++;

    uint256 _nonce = userNonces[msg.sender];

    relayed[_getTxHash(_nonce, _calls, msg.sender, _gasLimit)] = true;

    emit RelayedCalls(_nonce, msg.sender, _calls, _gasLimit);

    return _nonce;
  }

This way, the contract Can’t generate a DoS and if the attackers will only deny themself.

And this mapping mapping(uint256 => bool) public executed in CrossChainExecutor contracts should be change to:

mapping (address => mapping(uint256 => bool)) public executed;  

The text was updated successfully, but these errors were encountered:

All reactions