PoolTogether contest - enckrish's results

Lines of code

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

Vulnerability details

Impact

In CrossChainRelayerArbitrum.processCalls, msg.sender is set as the address to receive any refund (both excess fee refund and call value refund) in the createRetryableTicket call. Arbitrium credits the refunded value to L2 balance, so if the user doesn't control the refund address on L2, or the code at the address doesn't support ETH withdrawals, then the funds are lost. This situation is possible when the caller is a contract. It may be impossible to deploy a new contract at that same address on Arbitrium to claim the funds in some cases, such as:

1. The nonce of the deployer, required to deploy at that address has been surpassed.
2. In cases where the contract address is determined by CREATE2, a non-destructible contract already exists that doesn't support ETH withdrawals.

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

Proof of Concept

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

Tools Used

Manual Review

Recommended Mitigation Steps

processCalls may accept a new parameter refundAddress and pass that address as excessFeeRefundAddress and callValueRefundAddress in the createRetryableTicket call. This would make it the caller's responsibility to send a proper address for refund. Alternatively, processCalls call can be restricted to EOAs, although that would be very limiting for applications.