diff --git a/processor/ethereum/router/contracts/IRouter.sol b/processor/ethereum/router/contracts/IRouter.sol index 91bedac5..f3fab5d6 100644 --- a/processor/ethereum/router/contracts/IRouter.sol +++ b/processor/ethereum/router/contracts/IRouter.sol @@ -36,11 +36,15 @@ interface IRouterWithoutCollisions { error EscapeHatchInvoked(); /// @notice The signature was invalid error InvalidSignature(); + /// @notice The amount specified didn't match `msg.value` error AmountMismatchesMsgValue(); /// @notice The call to an ERC20's `transferFrom` failed error TransferFromFailed(); + /// @notice `execute` was re-entered + error ReenteredExecute(); + /// @notice An invalid address to escape to was specified. error InvalidEscapeAddress(); /// @notice Escaping when escape hatch wasn't invoked. diff --git a/processor/ethereum/router/contracts/Router.sol b/processor/ethereum/router/contracts/Router.sol index e8f54653..fbff77c9 100644 --- a/processor/ethereum/router/contracts/Router.sol +++ b/processor/ethereum/router/contracts/Router.sol @@ -25,6 +25,14 @@ import "IRouter.sol"; /// @author Luke Parker /// @notice Intakes coins for the Serai network and handles relaying batches of transfers out contract Router is IRouterWithoutCollisions { + /// @dev The address in transient storage used for the reentrancy guard + bytes32 constant EXECUTE_REENTRANCY_GUARD_SLOT = bytes32( + /* + keccak256("ReentrancyGuard Router.execute") - 1 + */ + 0xcf124a063de1614fedbd6b47187f98bf8873a1ae83da5c179a5881162f5b2401 + ); + /** * @dev The next nonce used to determine the address of contracts deployed with CREATE. This is * used to predict the addresses of deployed contracts ahead of time. @@ -356,6 +364,25 @@ contract Router is IRouterWithoutCollisions { // Each individual call is explicitly metered to ensure there isn't a DoS here // slither-disable-next-line calls-loop function execute4DE42904() external { + /* + Prevent re-entrancy. + + We emit a bitmask of which `OutInstruction`s succeeded. Doing that requires executing the + `OutInstruction`s, which may re-enter here. While our application of CEI with verifySignature + prevents replays, re-entrancy would allow out-of-order execution of batches (despite their + in-order start of execution) which isn't a headache worth dealing with. + */ + bytes32 executeReentrancyGuardSlot = EXECUTE_REENTRANCY_GUARD_SLOT; + bytes32 priorEntered; + // slither-disable-next-line assembly + assembly { + priorEntered := tload(executeReentrancyGuardSlot) + tstore(executeReentrancyGuardSlot, 1) + } + if (priorEntered != bytes32(0)) { + revert ReenteredExecute(); + } + (uint256 nonceUsed, bytes memory args, bytes32 message) = verifySignature(); (,, address coin, uint256 fee, IRouter.OutInstruction[] memory outs) = abi.decode(args, (bytes32, bytes32, address, uint256, IRouter.OutInstruction[])); diff --git a/processor/ethereum/router/src/tests/mod.rs b/processor/ethereum/router/src/tests/mod.rs index 78215d95..2da5422d 100644 --- a/processor/ethereum/router/src/tests/mod.rs +++ b/processor/ethereum/router/src/tests/mod.rs @@ -22,6 +22,18 @@ use ethereum_deployer::Deployer; use crate::{Coin, OutInstructions, Router}; +#[test] +fn execute_reentrancy_guard() { + let hash = alloy_core::primitives::keccak256(b"ReentrancyGuard Router.execute"); + assert_eq!( + alloy_core::primitives::hex::encode( + (U256::from_be_slice(hash.as_ref()) - U256::from(1u8)).to_be_bytes::<32>() + ), + // Constant from the Router contract + "cf124a063de1614fedbd6b47187f98bf8873a1ae83da5c179a5881162f5b2401", + ); +} + #[test] fn selector_collisions() { assert_eq!( @@ -32,6 +44,10 @@ fn selector_collisions() { crate::_irouter_abi::IRouter::updateSeraiKeyCall::SELECTOR, crate::_router_abi::Router::updateSeraiKey5A8542A2Call::SELECTOR ); + assert_eq!( + crate::_irouter_abi::IRouter::escapeHatchCall::SELECTOR, + crate::_router_abi::Router::escapeHatchDCDD91CCCall::SELECTOR + ); } pub(crate) fn test_key() -> (Scalar, PublicKey) {