diff --git a/test/concrete/MaliciousReenteringRecipient.sol b/test/concrete/MaliciousReenteringRecipient.sol new file mode 100644 index 00000000..8b43aabf --- /dev/null +++ b/test/concrete/MaliciousReenteringRecipient.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {IFlowV5} from "../../src/interface/IFlowV5.sol"; +import {EvaluableV2} from "rain.interpreter.interface/lib/caller/LibEvaluable.sol"; +import {SignedContextV1} from "rain.interpreter.interface/interface/IInterpreterCallerV2.sol"; +import {IERC721Receiver} from "openzeppelin-contracts/contracts/token/ERC721/IERC721Receiver.sol"; +import {IERC1155Receiver} from "openzeppelin-contracts/contracts/token/ERC1155/IERC1155Receiver.sol"; + +/// A recipient whose ERC721 / ERC1155 receiver hooks re-enter +/// `flow.flow(...)` on the same flow contract. Used to exercise +/// `Flow.flow`'s `nonReentrant` guard against reentry through the +/// EIP-721 / EIP-1155 receiver-side hooks. +contract MaliciousReenteringRecipient is IERC721Receiver, IERC1155Receiver { + IFlowV5 internal immutable I_FLOW; + EvaluableV2 internal evaluable; + + constructor(IFlowV5 flow_) { + I_FLOW = flow_; + } + + function setEvaluable(EvaluableV2 memory ev) external { + evaluable = ev; + } + + function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) { + I_FLOW.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + return IERC721Receiver.onERC721Received.selector; + } + + function onERC1155Received(address, address, uint256, uint256, bytes calldata) external returns (bytes4) { + I_FLOW.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + return IERC1155Receiver.onERC1155Received.selector; + } + + function onERC1155BatchReceived(address, address, uint256[] calldata, uint256[] calldata, bytes calldata) + external + returns (bytes4) + { + I_FLOW.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + return IERC1155Receiver.onERC1155BatchReceived.selector; + } + + function supportsInterface(bytes4 interfaceId) external pure returns (bool) { + return interfaceId == type(IERC721Receiver).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId + || interfaceId == 0x01ffc9a7; + } +} diff --git a/test/concrete/MaliciousReenteringStore.sol b/test/concrete/MaliciousReenteringStore.sol new file mode 100644 index 00000000..6afbb77a --- /dev/null +++ b/test/concrete/MaliciousReenteringStore.sol @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {IFlowV5} from "../../src/interface/IFlowV5.sol"; +import {EvaluableV2} from "rain.interpreter.interface/lib/caller/LibEvaluable.sol"; +import {SignedContextV1} from "rain.interpreter.interface/interface/IInterpreterCallerV2.sol"; + +/// An interpreter store whose `set` callback re-enters `flow.flow(...)` +/// on the same flow contract. Used to exercise `Flow.flow`'s +/// `nonReentrant` guard against reentry through the store's `set` path. +/// Its `get` returns zero so reads from this store do not crash other +/// callers. +contract MaliciousReenteringStore { + IFlowV5 internal immutable I_FLOW; + EvaluableV2 internal evaluable; + + constructor(IFlowV5 flow_) { + I_FLOW = flow_; + } + + function setEvaluable(EvaluableV2 memory ev) external { + evaluable = ev; + } + + /// Matches `IInterpreterStoreV2.set(StateNamespace, uint256[])`. Re-enters + /// the flow. + function set(uint256, uint256[] calldata) external { + I_FLOW.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + } + + function get(uint256, uint256) external pure returns (uint256) { + return 0; + } +} diff --git a/test/concrete/StubERC1155WithReceiverHook.sol b/test/concrete/StubERC1155WithReceiverHook.sol new file mode 100644 index 00000000..913f8a96 --- /dev/null +++ b/test/concrete/StubERC1155WithReceiverHook.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {IERC1155Receiver} from "openzeppelin-contracts/contracts/token/ERC1155/IERC1155Receiver.sol"; + +/// Minimal ERC1155-shaped stub that, on `safeTransferFrom`, calls the +/// recipient's `onERC1155Received` hook if the recipient has code. Used +/// in tests to exercise recipient-side reentrancy through the ERC1155 +/// path without deploying a full ERC1155 token implementation. +contract StubERC1155WithReceiverHook { + //forge-lint: disable-next-line(mixed-case-function) + function safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes calldata data) external { + if (to.code.length > 0) { + IERC1155Receiver(to).onERC1155Received(msg.sender, from, id, amount, data); + } + } +} diff --git a/test/concrete/StubERC721WithReceiverHook.sol b/test/concrete/StubERC721WithReceiverHook.sol new file mode 100644 index 00000000..b3315a52 --- /dev/null +++ b/test/concrete/StubERC721WithReceiverHook.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {IERC721Receiver} from "openzeppelin-contracts/contracts/token/ERC721/IERC721Receiver.sol"; + +/// Minimal ERC721-shaped stub that, on `safeTransferFrom`, calls the +/// recipient's `onERC721Received` hook if the recipient has code. Used in +/// tests to exercise recipient-side reentrancy through the ERC721 path +/// without deploying a full ERC721 token implementation. +contract StubERC721WithReceiverHook { + //forge-lint: disable-next-line(mixed-case-function) + function safeTransferFrom(address from, address to, uint256 tokenId) external { + if (to.code.length > 0) { + IERC721Receiver(to).onERC721Received(msg.sender, from, tokenId, ""); + } + } +} diff --git a/test/src/concrete/Flow.transfer.t.sol b/test/src/concrete/Flow.transfer.t.sol index 50c6e581..5e58881e 100644 --- a/test/src/concrete/Flow.transfer.t.sol +++ b/test/src/concrete/Flow.transfer.t.sol @@ -29,6 +29,10 @@ import {LibContextWrapper} from "test/lib/LibContextWrapper.sol"; import {IERC721} from "openzeppelin-contracts/contracts/token/ERC721/IERC721.sol"; import {IERC1155} from "openzeppelin-contracts/contracts/token/ERC1155/IERC1155.sol"; import {LibStackGeneration} from "test/lib/LibStackGeneration.sol"; +import {MaliciousReenteringStore} from "test/concrete/MaliciousReenteringStore.sol"; +import {MaliciousReenteringRecipient} from "test/concrete/MaliciousReenteringRecipient.sol"; +import {StubERC721WithReceiverHook} from "test/concrete/StubERC721WithReceiverHook.sol"; +import {StubERC1155WithReceiverHook} from "test/concrete/StubERC1155WithReceiverHook.sol"; /// `IERC721.safeTransferFrom` is overloaded (3-arg + 4-arg). Pin the 3-arg /// selector via a single-overload wrapper interface so the disambiguation @@ -428,4 +432,121 @@ contract FlowTransferTest is FlowTest { flow.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); vm.stopPrank(); } + + /// `Flow.flow` carries `nonReentrant`. A malicious interpreter store + /// whose `set` re-enters `flow.flow(...)` MUST cause the inner call + /// to revert. + function testFlowReentrancyGuardFiresOnStoreCallback(uint256 writeKey, uint256 writeValue) external { + address alice = address(uint160(uint256(keccak256("alice.reentrancy.store")))); + vm.label(alice, "Alice"); + + (IFlowV5 flow, EvaluableV2 memory evaluable) = deployFlow(); + assumeEtchable(alice, address(flow)); + + // Replace the framework's mock STORE with the malicious bytecode so + // LibFlow.flow's `set(...)` lands in the re-entering implementation. + MaliciousReenteringStore malStore = new MaliciousReenteringStore(flow); + malStore.setEvaluable(evaluable); + vm.etch(address(STORE), address(malStore).code); + // The runtime still needs to know which evaluable to flow into; the + // MaliciousReenteringStore reads its `evaluable` storage slot, so + // copy it over after the etch. + bytes32 evalSlot0 = vm.load(address(malStore), bytes32(uint256(0))); + bytes32 evalSlot1 = vm.load(address(malStore), bytes32(uint256(1))); + bytes32 evalSlot2 = vm.load(address(malStore), bytes32(uint256(2))); + vm.store(address(STORE), bytes32(uint256(0)), evalSlot0); + vm.store(address(STORE), bytes32(uint256(1)), evalSlot1); + vm.store(address(STORE), bytes32(uint256(2)), evalSlot2); + + // Eval2 returns non-empty kvs so LibFlow.flow takes the `set` branch. + uint256[] memory stack = LibStackGeneration.generateFlowStack(Sentinel.unwrap(RAIN_FLOW_SENTINEL), transferEmpty()); + uint256[] memory writes = new uint256[](2); + writes[0] = writeKey; + writes[1] = writeValue; + interpreterEval2MockCall(stack, writes); + + vm.startPrank(alice); + vm.expectRevert(bytes("ReentrancyGuard: reentrant call")); + flow.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + vm.stopPrank(); + } + + /// A malicious ERC721 recipient whose `onERC721Received` re-enters + /// `flow.flow(...)` MUST cause the inner call to revert. Exercises the + /// EIP-721 receiver-hook reentrancy path (the underlying token's + /// `safeTransferFrom` invokes the recipient hook on a contract `to`). + /// forge-config: default.fuzz.runs = 100 + function testFlowReentrancyGuardFiresOnERC721Recipient(uint256 tokenId) external { + vm.assume(Sentinel.unwrap(RAIN_FLOW_SENTINEL) != tokenId); + address alice = address(uint160(uint256(keccak256("alice.reentrancy.erc721")))); + vm.label(alice, "Alice"); + + (IFlowV5 flow, EvaluableV2 memory evaluable) = deployFlow(); + assumeEtchable(alice, address(flow)); + + // Etch a stub ERC721 implementation at TOKEN_B that forwards + // safeTransferFrom into the recipient hook. + StubERC721WithReceiverHook stub = new StubERC721WithReceiverHook(); + vm.etch(TOKEN_B, address(stub).code); + + // Deploy the malicious recipient and arm it with the flow's + // evaluable. + MaliciousReenteringRecipient recipient = new MaliciousReenteringRecipient(flow); + recipient.setEvaluable(evaluable); + + ERC721Transfer[] memory erc721Transfers = new ERC721Transfer[](1); + erc721Transfers[0] = + ERC721Transfer({token: TOKEN_B, from: address(flow), to: address(recipient), id: tokenId}); + + uint256[] memory stack = LibStackGeneration.generateFlowStack( + Sentinel.unwrap(RAIN_FLOW_SENTINEL), + FlowTransferV1(new ERC20Transfer[](0), erc721Transfers, new ERC1155Transfer[](0)) + ); + interpreterEval2MockCall(stack, new uint256[](0)); + + vm.startPrank(alice); + vm.expectRevert(bytes("ReentrancyGuard: reentrant call")); + flow.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + vm.stopPrank(); + } + + /// A malicious ERC1155 recipient whose `onERC1155Received` re-enters + /// `flow.flow(...)` MUST cause the inner call to revert. Exercises the + /// EIP-1155 receiver-hook reentrancy path. + /// forge-config: default.fuzz.runs = 100 + function testFlowReentrancyGuardFiresOnERC1155Recipient(uint256 tokenId, uint256 amount) external { + vm.assume(Sentinel.unwrap(RAIN_FLOW_SENTINEL) != tokenId); + vm.assume(Sentinel.unwrap(RAIN_FLOW_SENTINEL) != amount); + address alice = address(uint160(uint256(keccak256("alice.reentrancy.erc1155")))); + vm.label(alice, "Alice"); + + (IFlowV5 flow, EvaluableV2 memory evaluable) = deployFlow(); + assumeEtchable(alice, address(flow)); + + StubERC1155WithReceiverHook stub = new StubERC1155WithReceiverHook(); + vm.etch(TOKEN_C, address(stub).code); + + MaliciousReenteringRecipient recipient = new MaliciousReenteringRecipient(flow); + recipient.setEvaluable(evaluable); + + ERC1155Transfer[] memory erc1155Transfers = new ERC1155Transfer[](1); + erc1155Transfers[0] = ERC1155Transfer({ + token: TOKEN_C, + from: address(flow), + to: address(recipient), + id: tokenId, + amount: amount + }); + + uint256[] memory stack = LibStackGeneration.generateFlowStack( + Sentinel.unwrap(RAIN_FLOW_SENTINEL), + FlowTransferV1(new ERC20Transfer[](0), new ERC721Transfer[](0), erc1155Transfers) + ); + interpreterEval2MockCall(stack, new uint256[](0)); + + vm.startPrank(alice); + vm.expectRevert(bytes("ReentrancyGuard: reentrant call")); + flow.flow(evaluable, new uint256[](0), new SignedContextV1[](0)); + vm.stopPrank(); + } }