Skip to content

Commit

Permalink
test: demonstrate non-reentrancy guaranteed by CREATE2 address collision
Browse files Browse the repository at this point in the history
  • Loading branch information
ARR4N committed May 21, 2024
1 parent 56e767d commit 08c1ad0
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 0 deletions.
6 changes: 6 additions & 0 deletions test/ERC721ForERC20.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {ERC20Test} from "./ERC20Test.t.sol";

import {ERC721Token} from "../src/ERC721SwapperLib.sol";
import {ERC721ForERC20Swap, IERC20} from "../src/ERC721ForERC20/ERC721ForERC20Swap.sol";
import {ERC721ForERC20SwapperDeployer} from "../src/ERC721ForERC20/ERC721ForERC20SwapperDeployer.gen.sol";
import {InsufficientBalance, Disbursement, Parties} from "../src/TypesAndConstants.sol";

import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
Expand Down Expand Up @@ -50,6 +51,11 @@ contract ERC721ForERC20Test is ERC721ForXTest, ERC20Test {
return abi.encode(_asSwap(t), salt);
}

/// @inheritdoc ERC721ForXTest
function _fillSelector() internal pure override returns (bytes4) {
return ERC721ForERC20SwapperDeployer.fill.selector;
}

/// @inheritdoc ERC721ForXTest
function _fill(ERC721TestCase memory t) internal override {
factory.fill(_asSwap(t), t.base.salt);
Expand Down
6 changes: 6 additions & 0 deletions test/ERC721ForNative.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {NativeTokenTest} from "./NativeTokenTest.t.sol";

import {ERC721Token} from "../src/ERC721SwapperLib.sol";
import {ERC721ForNativeSwap} from "../src/ERC721ForNative/ERC721ForNativeSwap.sol";
import {ERC721ForNativeSwapperDeployer} from "../src/ERC721ForNative/ERC721ForNativeSwapperDeployer.gen.sol";
import {InsufficientBalance, Disbursement, Parties} from "../src/TypesAndConstants.sol";

import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
Expand Down Expand Up @@ -44,6 +45,11 @@ contract ERC721ForNativeTest is ERC721ForXTest, NativeTokenTest {
return abi.encode(_asSwap(t), salt);
}

/// @inheritdoc ERC721ForXTest
function _fillSelector() internal pure override returns (bytes4) {
return ERC721ForNativeSwapperDeployer.fill.selector;
}

/// @inheritdoc ERC721ForXTest
function _fill(ERC721TestCase memory t) internal override {
_fill(t, t.base.native.callValue);
Expand Down
19 changes: 19 additions & 0 deletions test/ERC721ForXTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ abstract contract ERC721ForXTest is SwapperTestBase {

function _encodedSwapAndSalt(ERC721TestCase memory, bytes32) internal view virtual returns (bytes memory);

function _fillSelector() internal pure virtual returns (bytes4);

/// @dev Fills the swap defined by the test case.
function _fill(ERC721TestCase memory) internal virtual;

Expand Down Expand Up @@ -278,6 +280,23 @@ abstract contract ERC721ForXTest is SwapperTestBase {
vm.stopPrank();
}

function testNonReentrant(ERC721TestCase memory t)
external
assumeValidTest(t.base)
assumePaymentsValid(t.base)
assumeSufficientPayment(t.base)
assumeValidPlatformFee(t.base)
assumeApproving(t.base)
{
token.setPostTransferCall(
address(factory), abi.encodePacked(_fillSelector(), _encodedSwapAndSalt(t, t.base.salt))
);

// The most precise way to detect a redeployment is to see that CREATE2 reverts without any return data.
// Inspection of the trace with `forge test -vvvv` is necessary to see the [CreationCollision] error.
_testFill(t, abi.encodeWithSelector(ETDeployer.Create2EmptyRevert.selector));
}

function testGas() external {
Disbursement[5] memory thirdParty;
uint128 total = 1 ether;
Expand Down
6 changes: 6 additions & 0 deletions test/MultiERC721ForERC20.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {ERC20Test} from "./ERC20Test.t.sol";

import {MultiERC721Token} from "../src/ERC721SwapperLib.sol";
import {MultiERC721ForERC20Swap, IERC20} from "../src/MultiERC721ForERC20/MultiERC721ForERC20Swap.sol";
import {MultiERC721ForERC20SwapperDeployer} from "../src/MultiERC721ForERC20/MultiERC721ForERC20SwapperDeployer.gen.sol";
import {InsufficientBalance, Disbursement, Parties} from "../src/TypesAndConstants.sol";

import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
Expand Down Expand Up @@ -58,6 +59,11 @@ contract MultiERC721ForERC20Test is ERC721ForXTest, ERC20Test {
return abi.encode(_asSwap(t), salt);
}

/// @inheritdoc ERC721ForXTest
function _fillSelector() internal pure override returns (bytes4) {
return MultiERC721ForERC20SwapperDeployer.fill.selector;
}

/// @inheritdoc ERC721ForXTest
function _fill(ERC721TestCase memory t) internal override {
factory.fill(_asSwap(t), t.base.salt);
Expand Down
7 changes: 7 additions & 0 deletions test/MultiERC721ForNative.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {NativeTokenTest} from "./NativeTokenTest.t.sol";

import {MultiERC721Token} from "../src/ERC721SwapperLib.sol";
import {MultiERC721ForNativeSwap} from "../src/MultiERC721ForNative/MultiERC721ForNativeSwap.sol";
import {MultiERC721ForNativeSwapperDeployer} from
"../src/MultiERC721ForNative/MultiERC721ForNativeSwapperDeployer.gen.sol";
import {InsufficientBalance, Disbursement, Parties} from "../src/TypesAndConstants.sol";

import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
Expand Down Expand Up @@ -52,6 +54,11 @@ contract MultiERC721ForNativeTest is ERC721ForXTest, NativeTokenTest {
return abi.encode(_asSwap(t), salt);
}

/// @inheritdoc ERC721ForXTest
function _fillSelector() internal pure override returns (bytes4) {
return MultiERC721ForNativeSwapperDeployer.fill.selector;
}

/// @inheritdoc ERC721ForXTest
function _fill(ERC721TestCase memory t) internal override {
_fill(t, t.base.native.callValue);
Expand Down
23 changes: 23 additions & 0 deletions test/SwapperTestBase.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {

import {ERC721, IERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol";
import {Address} from "@openzeppelin/contracts/utils/Address.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

contract Token is ERC721 {
Expand All @@ -28,6 +29,28 @@ contract Token is ERC721 {
function exists(uint256 tokenId) external view returns (bool) {
return _ownerOf(tokenId) != address(0);
}

/**
* @dev If non-zero, this address is called after transferFrom() to enable testing of reentrancy. Although an attack
* would typically be performed by a Party, the token transfer is the only common function call in all tests so is
* the cleanest way to insert a reentrancy hook.
*/
address private _callPostTransfer;

bytes private _postTransferCallData;

function setPostTransferCall(address a, bytes calldata data) public {
_callPostTransfer = a;
_postTransferCallData = data;
}

function transferFrom(address from, address to, uint256 tokenId) public override {
super.transferFrom(from, to, tokenId);

if (_callPostTransfer != address(0)) {
Address.functionCall(_callPostTransfer, _postTransferCallData);
}
}
}

interface ITestEvents is ISwapperEvents {
Expand Down

0 comments on commit 08c1ad0

Please sign in to comment.