From 0484113254b12578ff1d464a39247367f7c14630 Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Mon, 27 Jan 2025 13:07:35 -0500
Subject: [PATCH] Fix the ability for a malicious adversary to snipe ERC20s out
 via re-entrancy from the ERC20 contract

---
 .../ethereum/router/contracts/IRouter.sol     |  2 ++
 .../ethereum/router/contracts/Router.sol      | 24 ++++++++++++++++---
 processor/ethereum/router/src/tests/mod.rs    | 20 ++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/processor/ethereum/router/contracts/IRouter.sol b/processor/ethereum/router/contracts/IRouter.sol
index 772a04f7..1cf61f8e 100644
--- a/processor/ethereum/router/contracts/IRouter.sol
+++ b/processor/ethereum/router/contracts/IRouter.sol
@@ -63,6 +63,8 @@ interface IRouterWithoutCollisions {
   /// @notice The call to an ERC20's `transferFrom` failed
   error TransferFromFailed();
 
+  /// @notice The code wasn't to-be-executed by self
+  error CodeNotBySelf();
   /// @notice A non-reentrant function was re-entered
   error Reentered();
 
diff --git a/processor/ethereum/router/contracts/Router.sol b/processor/ethereum/router/contracts/Router.sol
index 25ece4b8..dd0ad6e5 100644
--- a/processor/ethereum/router/contracts/Router.sol
+++ b/processor/ethereum/router/contracts/Router.sol
@@ -406,9 +406,8 @@ contract Router is IRouterWithoutCollisions {
         arbitrarily called). We accordingly don't need to be worried about return bombs here.
       */
       // slither-disable-next-line return-bomb
-      (bool erc20Success, bytes memory res) = address(coin).call{ gas: ERC20_GAS }(
-        abi.encodeWithSelector(selector, to, amount)
-      );
+      (bool erc20Success, bytes memory res) =
+        address(coin).call{ gas: ERC20_GAS }(abi.encodeWithSelector(selector, to, amount));
 
       /*
         Require there was nothing returned, which is done by some non-standard tokens, or that the
@@ -504,6 +503,25 @@ contract Router is IRouterWithoutCollisions {
    */
   /// @param code The code to execute
   function executeArbitraryCode(bytes memory code) external payable {
+    /*
+      execute assumes that from the time it reads `_smartContractNonce` until the time it calls this
+      function, no mutations to it will occur. If any mutations could occur, it'd lead to a fault
+      where tokens could be sniped by:
+
+      1) An out occurring, transferring tokens to an about-to-be-deployed smart contract
+      2) The token contract re-entering the Router to deploy a new smart contract which claims the
+         tokens
+      3) The Router then deploying the intended smart contract (ignoring whatever result it may
+         have)
+
+      This does assume a malicious token, or a token with callbacks which can be set by a malicious
+      adversary, yet the way to ensure it's a non-issue is to not allow other entities to mutate
+      `_smartContractNonce`.
+    */
+    if (msg.sender != address(this)) {
+      revert CodeNotBySelf();
+    }
+
     // Because we're creating a contract, increment our nonce
     _smartContractNonce += 1;
 
diff --git a/processor/ethereum/router/src/tests/mod.rs b/processor/ethereum/router/src/tests/mod.rs
index 4c21aeaf..2f6397cc 100644
--- a/processor/ethereum/router/src/tests/mod.rs
+++ b/processor/ethereum/router/src/tests/mod.rs
@@ -706,6 +706,26 @@ async fn test_erc20_top_level_transfer_in_instruction() {
   test.publish_in_instruction_tx(tx, coin, amount, &shorthand).await;
 }
 
+#[tokio::test]
+async fn test_execute_arbitrary_code() {
+  let test = Test::new().await;
+
+  assert!(matches!(
+    test
+      .call_and_decode_err(TxLegacy {
+        chain_id: None,
+        nonce: 0,
+        gas_price: 100_000_000_000,
+        gas_limit: 1_000_000,
+        to: test.router.address().into(),
+        value: U256::ZERO,
+        input: crate::abi::executeArbitraryCodeCall::new((vec![].into(),)).abi_encode().into(),
+      })
+      .await,
+    IRouterErrors::CodeNotBySelf(IRouter::CodeNotBySelf {})
+  ));
+}
+
 // Code which returns true
 #[rustfmt::skip]
 fn return_true_code() -> Vec<u8> {