Add a re-entrancy guard to Router.execute

This commit is contained in:
Luke Parker
2024-11-02 20:12:48 -04:00
parent 26230377b0
commit 2920987173
3 changed files with 47 additions and 0 deletions

View File

@@ -36,11 +36,15 @@ interface IRouterWithoutCollisions {
error EscapeHatchInvoked(); error EscapeHatchInvoked();
/// @notice The signature was invalid /// @notice The signature was invalid
error InvalidSignature(); error InvalidSignature();
/// @notice The amount specified didn't match `msg.value` /// @notice The amount specified didn't match `msg.value`
error AmountMismatchesMsgValue(); error AmountMismatchesMsgValue();
/// @notice The call to an ERC20's `transferFrom` failed /// @notice The call to an ERC20's `transferFrom` failed
error TransferFromFailed(); error TransferFromFailed();
/// @notice `execute` was re-entered
error ReenteredExecute();
/// @notice An invalid address to escape to was specified. /// @notice An invalid address to escape to was specified.
error InvalidEscapeAddress(); error InvalidEscapeAddress();
/// @notice Escaping when escape hatch wasn't invoked. /// @notice Escaping when escape hatch wasn't invoked.

View File

@@ -25,6 +25,14 @@ import "IRouter.sol";
/// @author Luke Parker <lukeparker@serai.exchange> /// @author Luke Parker <lukeparker@serai.exchange>
/// @notice Intakes coins for the Serai network and handles relaying batches of transfers out /// @notice Intakes coins for the Serai network and handles relaying batches of transfers out
contract Router is IRouterWithoutCollisions { 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 * @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. * 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 // Each individual call is explicitly metered to ensure there isn't a DoS here
// slither-disable-next-line calls-loop // slither-disable-next-line calls-loop
function execute4DE42904() external { 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(); (uint256 nonceUsed, bytes memory args, bytes32 message) = verifySignature();
(,, address coin, uint256 fee, IRouter.OutInstruction[] memory outs) = (,, address coin, uint256 fee, IRouter.OutInstruction[] memory outs) =
abi.decode(args, (bytes32, bytes32, address, uint256, IRouter.OutInstruction[])); abi.decode(args, (bytes32, bytes32, address, uint256, IRouter.OutInstruction[]));

View File

@@ -22,6 +22,18 @@ use ethereum_deployer::Deployer;
use crate::{Coin, OutInstructions, Router}; 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] #[test]
fn selector_collisions() { fn selector_collisions() {
assert_eq!( assert_eq!(
@@ -32,6 +44,10 @@ fn selector_collisions() {
crate::_irouter_abi::IRouter::updateSeraiKeyCall::SELECTOR, crate::_irouter_abi::IRouter::updateSeraiKeyCall::SELECTOR,
crate::_router_abi::Router::updateSeraiKey5A8542A2Call::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) { pub(crate) fn test_key() -> (Scalar, PublicKey) {