Reentrancy Attack

[code423n4]

Lines of code

https://github.com/pooltogether/ERC5164/blob/5647bd84f2a6d1a37f41394874d567e45a97bf48/src/ethereum-arbitrum/EthereumToArbitrumRelayer.sol#L101-L132

Vulnerability details

Impact

By using the relayCalls function, a malicious user can call the processCalls function with the same parameters more than once without increasing the nonce value. This can be exploited by the attacker to execute the same calls multiple times.

Proof of Concept

There is a potential reentrancy vulnerability in the processCalls function.

1. The function calls the inbox.createRetryableTicket function, which is a payable function,
2. and it passes the msg.sender address as one of the parameters.
3. This allows an attacker to call processCalls with a malicious contract address and then call the inbox.createRetryableTicket function to reenter the processCalls function.

Tools Used

Slither, Echidna, MythX

Recommended Mitigation Steps

1. To fix this bug, we can add a require(msg.sender == user) or
2. require(!msg.sender.call.value(0)()) statement at the beginning of the processCalls function.

function processCalls(
    uint256 _nonce,
    CallLib.Call[] calldata _calls,
    address _sender,
    uint256 _gasLimit,
    uint256 _maxSubmissionCost,
    uint256 _gasPriceBid
  ) external payable returns (uint256) {
    require(!msg.sender.call.value(0)());
    require(relayed[_getTxHash(_nonce, _calls, _sender, _gasLimit)], "Relayer/calls-not-relayed");

    bytes memory _data = abi.encodeWithSignature(
      "executeCalls(uint256,address,(address,bytes)[])",
      _nonce,
      _sender,
      _calls
    );

    uint256 _ticketID = inbox.createRetryableTicket{ value: msg.value }(
      address(executor),
      0,
      _maxSubmissionCost,
      msg.sender,
      msg.sender,
      _gasLimit,
      _gasPriceBid,
      _data
    );

    emit ProcessedCalls(_nonce, msg.sender, _ticketID);

    return _ticketID;
  }

[GalloDaSballo]

Missing POC. Talks about reEntering but the only thing that happens after the call is an event.

Closing for lack of Proof

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof