[to consider] dirty reentrancy lock

contracts/UniversalRouter.sol

@@ -7,8 +7,9 @@ import {RouterParameters, RouterImmutables} from './base/RouterImmutables.sol';
 import {Constants} from './libraries/Constants.sol';
 import {Commands} from './libraries/Commands.sol';
 import {IUniversalRouter} from './interfaces/IUniversalRouter.sol';
+import {ReentrancyLock} from './base/ReentrancyLock.sol';
 
-contract UniversalRouter is RouterImmutables, IUniversalRouter, Dispatcher, RewardsCollector {
+contract UniversalRouter is RouterImmutables, IUniversalRouter, Dispatcher, RewardsCollector, ReentrancyLock {
     modifier checkDeadline(uint256 deadline) {
         if (block.timestamp > deadline) revert TransactionDeadlinePassed();
         _;
@@ -26,7 +27,7 @@ contract UniversalRouter is RouterImmutables, IUniversalRouter, Dispatcher, Rewa
     }
 
     /// @inheritdoc IUniversalRouter
-    function execute(bytes calldata commands, bytes[] calldata inputs) public payable {
+    function execute(bytes calldata commands, bytes[] calldata inputs) public payable isNotLocked {
         bool success;
         bytes memory output;
         uint256 numCommands = commands.length;

contracts/base/ReentrancyLock.sol

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity ^0.8.17;
+
+contract ReentrancyLock {
+    error ContractLocked();
+
+    uint256 private isLocked = 1;
+
+    modifier isNotLocked() {
+        if (isLocked != 1) revert ContractLocked();
+        isLocked = 2;
+        _;
+        isLocked = 1;
+    }
+}

contracts/test/ReenteringProtocol.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity ^0.8.15;
+
+contract ReenteringProtocol {
+    error NotAllowedReenter();
+
+    function callAndReenter(address universalRouter, bytes calldata data) public payable {
+        (bool success,) = universalRouter.call(data);
+        if (!success) revert NotAllowedReenter();
+    }
+}

test/integration-tests/UniversalRouter.test.ts

@@ -2,6 +2,7 @@ import { UniversalRouter, Permit2, ERC20, MockLooksRareRewardsDistributor, ERC72
 import { BigNumber, BigNumberish } from 'ethers'
 import { Pair } from '@uniswap/v2-sdk'
 import { expect } from './shared/expect'
+import { abi as ROUTER_ABI } from '../../artifacts/contracts/UniversalRouter.sol/UniversalRouter.json'
 import { abi as TOKEN_ABI } from '../../artifacts/solmate/tokens/ERC20.sol/ERC20.json'
 import NFTX_ZAP_ABI from './shared/abis/NFTXZap.json'
 import deployUniversalRouter, { deployPermit2 } from './shared/deployUniversalRouter'
@@ -16,6 +17,7 @@ import {
   SOURCE_MSG_SENDER,
   MAX_UINT160,
   MAX_UINT,
+  ETH_ADDRESS,
 } from './shared/constants'
 import {
   seaportOrders,
@@ -34,6 +36,7 @@ import hre from 'hardhat'
 
 const { ethers } = hre
 const nftxZapInterface = new ethers.utils.Interface(NFTX_ZAP_ABI)
+const routerInterface = new ethers.utils.Interface(ROUTER_ABI)
 
 describe('UniversalRouter', () => {
   let alice: SignerWithAddress
@@ -131,6 +134,37 @@ describe('UniversalRouter', () => {
       await expect(router['execute(bytes,bytes[])'](commands, inputs)).to.be.revertedWith('InvalidBips()')
     })
 
+    it('reverts if a malicious contract tries to reenter', async () => {
+      const reentrantProtocol = await (await ethers.getContractFactory('ReenteringProtocol')).deploy()
+
+      router = (
+        await deployUniversalRouter(
+          permit2,
+          mockLooksRareRewardsDistributor.address,
+          mockLooksRareToken.address,
+          reentrantProtocol.address
+        )
+      ).connect(alice) as UniversalRouter
+
+      planner.addCommand(CommandType.SWEEP, [ETH_ADDRESS, alice.address, 0])
+      let { commands, inputs } = planner
+
+      const sweepCalldata = routerInterface.encodeFunctionData('execute(bytes,bytes[])', [commands, inputs])
+      const reentrantCalldata = reentrantProtocol.interface.encodeFunctionData('callAndReenter', [
+        router.address,
+        sweepCalldata,
+      ])
+
+      planner = new RoutePlanner()
+      planner.addCommand(CommandType.NFTX, [0, reentrantCalldata])
+      ;({ commands, inputs } = planner)
+
+      const notAllowedReenterSelector = '0xb418cb98'
+      await expect(router['execute(bytes,bytes[])'](commands, inputs)).to.be.revertedWith(
+        `ExecutionFailed(0, "` + notAllowedReenterSelector + `")`
+      )
+    })
+
     describe('partial fills', async () => {
       let nftxValue: BigNumber
       let numCovens: number

test/integration-tests/gas-tests/__snapshots__/CheckOwnership.gas.test.ts.snap

@@ -3,27 +3,27 @@
 exports[`Check Ownership Gas gas: checks ownership after a seaport trade, one NFT 1`] = `
 Object {
   "calldataByteLength": 2212,
-  "gasUsed": 203708,
+  "gasUsed": 206020,
 }
 `;
 
 exports[`Check Ownership Gas gas: checks ownership after a seaport trade, two NFTs 1`] = `
 Object {
   "calldataByteLength": 5028,
-  "gasUsed": 339196,
+  "gasUsed": 341508,
 }
 `;
 
 exports[`Check Ownership Gas gas: does not check ownership after a seaport trade, one NFT 1`] = `
 Object {
   "calldataByteLength": 2052,
-  "gasUsed": 200173,
+  "gasUsed": 202485,
 }
 `;
 
 exports[`Check Ownership Gas gas: just ownership check 1`] = `
 Object {
   "calldataByteLength": 356,
-  "gasUsed": 30615,
+  "gasUsed": 32927,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/CryptoPunk.gas.test.ts.snap

@@ -3,6 +3,6 @@
 exports[`Cryptopunks purchases 1 cryptopunk gas 1`] = `
 Object {
   "calldataByteLength": 356,
-  "gasUsed": 105779,
+  "gasUsed": 109868,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/Foundation.gas.test.ts.snap

@@ -3,6 +3,6 @@
 exports[`Foundation Gas Tests Buy a mental worlds NFT from Foundation gas: token id 32 of mental worlds 1`] = `
 Object {
   "calldataByteLength": 612,
-  "gasUsed": 179658,
+  "gasUsed": 183748,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/LooksRare.gas.test.ts.snap

@@ -3,13 +3,13 @@
 exports[`LooksRare Gas Tests ERC-721 Purchase gas: buy 1 ERC-721 on looks rare 1`] = `
 Object {
   "calldataByteLength": 1316,
-  "gasUsed": 246469,
+  "gasUsed": 248781,
 }
 `;
 
 exports[`LooksRare Gas Tests ERC-1155 Purchase gas: buy 1 ERC-1155 on looks rare 1`] = `
 Object {
   "calldataByteLength": 1348,
-  "gasUsed": 266494,
+  "gasUsed": 268806,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/NFT20.gas.test.ts.snap

@@ -3,6 +3,6 @@
 exports[`NFT20 Buy 3 alphabetties from NFT20 gas: purchases token ids 129, 193, 278 of Alphabetties 1`] = `
 Object {
   "calldataByteLength": 836,
-  "gasUsed": 391360,
+  "gasUsed": 393672,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/NFTX.gas.test.ts.snap

@@ -3,6 +3,6 @@
 exports[`NFTX Gas Tests gas: buyAndRedeem w/ specific selection 1`] = `
 Object {
   "calldataByteLength": 740,
-  "gasUsed": 490835,
+  "gasUsed": 494924,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/Payments.gas.test.ts.snap

@@ -3,48 +3,48 @@
 exports[`Payments Gas Tests Individual Command Tests gas: SWEEP with ERC20 1`] = `
 Object {
   "calldataByteLength": 356,
-  "gasUsed": 37194,
+  "gasUsed": 39506,
 }
 `;
 
 exports[`Payments Gas Tests Individual Command Tests gas: SWEEP_WITH_FEE 1`] = `
 Object {
   "calldataByteLength": 516,
-  "gasUsed": 66436,
+  "gasUsed": 68748,
 }
 `;
 
 exports[`Payments Gas Tests Individual Command Tests gas: TRANSFER with ERC20 1`] = `
 Object {
   "calldataByteLength": 356,
-  "gasUsed": 36168,
+  "gasUsed": 38480,
 }
 `;
 
 exports[`Payments Gas Tests Individual Command Tests gas: TRANSFER with ETH 1`] = `
 Object {
   "calldataByteLength": 356,
-  "gasUsed": 31759,
+  "gasUsed": 34071,
 }
 `;
 
 exports[`Payments Gas Tests Individual Command Tests gas: UNWRAP_WETH 1`] = `
 Object {
   "calldataByteLength": 324,
-  "gasUsed": 44647,
+  "gasUsed": 46959,
 }
 `;
 
 exports[`Payments Gas Tests Individual Command Tests gas: UNWRAP_WETH_WITH_FEE 1`] = `
 Object {
   "calldataByteLength": 644,
-  "gasUsed": 52003,
+  "gasUsed": 54315,
 }
 `;
 
 exports[`Payments Gas Tests Individual Command Tests gas: WRAP_ETH 1`] = `
 Object {
   "calldataByteLength": 324,
-  "gasUsed": 53491,
+  "gasUsed": 57580,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/Seaport.gas.test.ts.snap

@@ -3,13 +3,13 @@
 exports[`Seaport Gas Tests gas: fulfillAdvancedOrder 1`] = `
 Object {
   "calldataByteLength": 2052,
-  "gasUsed": 200173,
+  "gasUsed": 202485,
 }
 `;
 
 exports[`Seaport Gas Tests gas: fulfillAvailableAdvancedOrders 1 orders 1`] = `
 Object {
   "calldataByteLength": 4708,
-  "gasUsed": 332120,
+  "gasUsed": 334432,
 }
 `;

test/integration-tests/gas-tests/__snapshots__/Sudoswap.gas.test.ts.snap

@@ -3,6 +3,6 @@
 exports[`Sudoswap Gas Tests Buy 3 sudolets from sudoswap gas: purchases token ids 80, 35, 93 of Sudolets 1`] = `
 Object {
   "calldataByteLength": 836,
-  "gasUsed": 190323,
+  "gasUsed": 192635,
 }
 `;