From be8a8f244f5cc04cbd5d83dd766cead1221160f9 Mon Sep 17 00:00:00 2001 From: dev-agent Date: Fri, 5 Jun 2026 23:39:24 +0800 Subject: [PATCH 1/2] fix: resolve setup friction and Foundry compatibility issues - ReentrancyCheck: remove redundant vm.prank that caused stacking error in Foundry >= 1.0 ('cannot overwrite a prank until applied'). - README: add Prerequisites section documenting GLIBC requirement and Docker fallback for older Linux distros, plus Slither memory-saving invocation tips. Verified on Linux x86_64 (glibc 2.32, Foundry 1.7.1 via Docker). Closes #35. --- .gitignore | 1 + README.md | 22 ++++++++++++++++++++++ src/checks/ReentrancyCheck.sol | 3 +-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 5cc2091..559a0ec 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ cache/ lib/ node_modules/ .env +**/__pycache__/ diff --git a/README.md b/README.md index 5507301..4e1d02b 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,28 @@ forge install kcolbchain/audit-checklist When working on this repository directly, initialize submodules before running the local verification commands: +### Prerequisites + +- **Foundry** (forge + cast): Install via [foundryup](https://book.getfoundry.sh/getting-started/installation). + - ⚠️ **Linux users**: The latest forge binaries require **GLIBC >= 2.33**. + Run `ldd --version` to check. If you're on an older distro (e.g. CentOS 8, + Ubuntu 20.04 with stock glibc), use the [Foundry Docker image](https://ghcr.io/foundry-rs/foundry): + ```bash + docker pull ghcr.io/foundry-rs/foundry:latest + docker run --rm -v "$PWD":/workspace -w /workspace ghcr.io/foundry-rs/foundry:latest forge test + ``` +- **Slither** (optional, for static analysis): `pip install slither-analyzer` + - Memory-constrained hosts can run specific detectors: + ```bash + slither . --config slither.config.json --detect reentrancy-eth,uninitialized-state,unchecked-transfer + ``` + - Full analysis (may need 4 GB+ RAM): + ```bash + slither . --config slither.config.json + ``` + +### Quick verification + ```bash git submodule update --init --recursive forge build diff --git a/src/checks/ReentrancyCheck.sol b/src/checks/ReentrancyCheck.sol index 23b4f50..3657664 100644 --- a/src/checks/ReentrancyCheck.sol +++ b/src/checks/ReentrancyCheck.sol @@ -28,14 +28,13 @@ abstract contract ReentrancyCheck is ChecklistBase { vm.deal(attacker, depositAmount); // Deposit as the attacker - vm.prank(attacker); + // NOTE: performDeposit must handle msg.sender pranking internally. performDeposit(attacker, depositAmount); uint256 targetBalBefore = address(targetContract).balance; // Trigger the attack — attacker calls withdraw, which sends ETH, // which triggers receive(), which re-enters withdraw() - vm.prank(attacker); ReentrantAttacker(payable(attacker)).attack(); uint256 attackerBal = address(attacker).balance; From 909f5a1c622c2d608a36de665b1e4c8bdf2ca122 Mon Sep 17 00:00:00 2001 From: dev-agent Date: Sat, 6 Jun 2026 07:05:48 +0800 Subject: [PATCH 2/2] fix: add explicit @notice to performDeposit about msg.sender pranking - ReentrancyCheck.performDeposit: clarify in NatSpec that implementations MUST use vm.prank(depositor) before calling the deposit function - ERC777ReentrancyCheck.performDeposit: same NatSpec clarification - Inline comment explains why no vm.prank before performDeposit call (avoids prank-stacking error in Foundry >= 1.0) --- src/checks/ERC777ReentrancyCheck.sol | 2 ++ src/checks/ReentrancyCheck.sol | 13 ++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/checks/ERC777ReentrancyCheck.sol b/src/checks/ERC777ReentrancyCheck.sol index 2a98d99..dc81f6b 100644 --- a/src/checks/ERC777ReentrancyCheck.sol +++ b/src/checks/ERC777ReentrancyCheck.sol @@ -29,6 +29,8 @@ abstract contract ERC777ReentrancyCheck is ChecklistBase { /// @dev Perform a deposit as `depositor` of `amount`. Override to match /// your target's deposit flow (approve+pull, push, etc.). + /// @notice Implementations MUST use `vm.prank(depositor)` (or `vm.startPrank`/`vm.stopPrank`) + /// before the deposit call so that the target contract sees `depositor` as `msg.sender`. function performDeposit(address depositor, uint256 amount) internal virtual; function test_erc777_reentrancy_on_withdraw() public { diff --git a/src/checks/ReentrancyCheck.sol b/src/checks/ReentrancyCheck.sol index 3657664..9ad09db 100644 --- a/src/checks/ReentrancyCheck.sol +++ b/src/checks/ReentrancyCheck.sol @@ -16,7 +16,11 @@ abstract contract ReentrancyCheck is ChecklistBase { return 1 ether; } - /// @dev Override to perform a deposit into the target contract + /// @dev Override to perform a deposit into the target contract as `depositor`. + /// @notice Implementations MUST use `vm.prank(depositor)` (or `vm.startPrank`/`vm.stopPrank`) + /// before the deposit call so that the target contract sees `depositor` as `msg.sender`. + /// The check function does NOT call `vm.prank` before this hook to avoid prank-stacking + /// errors in Foundry >= 1.0. function performDeposit(address depositor, uint256 amount) internal virtual; function test_reentrancy_on_withdraw() public { @@ -27,8 +31,11 @@ abstract contract ReentrancyCheck is ChecklistBase { uint256 depositAmount = getDepositValue(); vm.deal(attacker, depositAmount); - // Deposit as the attacker - // NOTE: performDeposit must handle msg.sender pranking internally. + // Deposit as the attacker. + // NOTE: performDeposit handles msg.sender internally via vm.prank. + // Do NOT add vm.prank(attacker) here — it would stack with the prank + // inside performDeposit and cause Foundry >= 1.0 to throw + // "cannot overwrite a prank until it is applied at least once". performDeposit(attacker, depositAmount); uint256 targetBalBefore = address(targetContract).balance;