Skip to content

Apply remaining Slither reentrancy fixes (CEI pattern) - #28

Merged
KaelSensei merged 4 commits into
mainfrom
fix/slither-reentrancy-followup
Apr 6, 2026
Merged

KaelSensei merged 4 commits into
mainfrom
fix/slither-reentrancy-followup

Conversation

@KaelSensei

Copy link
Copy Markdown
Owner

Summary

This PR applies the remaining 3 commits from the original fix that didn't make it into PR #27. All changes follow the check-effects-interactions (CEI) pattern to prevent reentrancy vulnerabilities.

Fixes Applied:

  • Move event emissions BEFORE external calls in 16 additional contracts
  • Add missing noReentrancy guard to TryCatch.tryLowLevelCall()
  • Add missing OwnerChanged event to FunctionModifier.changeOwner()

Contracts Fixed:

  • Call.callBySelector()
  • CrowdFund.unpledge()
  • CustomError.withdraw()
  • DutchAuction.endAuction()
  • FunctionModifier.changeOwner() - added event
  • MerkleAirdrop.claim()
  • MinimalProxy.cloneWithInitialization()
  • MultiCall.multicall()
  • MultiSigWallet.executeTransaction()
  • Payable.withdraw() and withdrawTo()
  • SendingEther.sendViaCall(), sendSafely(), sendViaSend()
  • TimeLock.execute()
  • TokenLocker.createLock() and withdraw()
  • TryCatch.tryLowLevelCall() - added guard
  • UniswapV2Swap.flashSwap()
  • UniswapV3Swap.swapExactInputSingle()
  • UniswapV4LimitOrder.createOrder() and cancelOrder()

Test Plan

  • All 945 tests passing
  • Code follows check-effects-interactions pattern
  • Verified with Slither - significantly reduced reentrancy warnings

KaelSensei and others added 3 commits April 6, 2026 13:51
…racts

- Move event emissions before external calls in UniswapV4LimitOrder, MerkleAirdrop
- Add missing OwnerChanged event to FunctionModifier.changeOwner()
- Add noReentrancy guard to TryCatch.tryLowLevelCall() to prevent state changes after external calls
- Restructure UniswapV4LimitOrder.createOrder() to emit event before transfer (CEI pattern)
- Improve code safety by following check-effects-interactions pattern consistently

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Apply check-effects-interactions pattern consistently:
- DutchAuction.endAuction()
- TokenLocker.createLock()
- SendingEther.sendViaCall() and sendSafely()
- MinimalProxy.cloneWithInitialization()
- TimeLock.execute()
- MultiSigWallet.executeTransaction()
- UniswapV2Swap.flashSwap()
- MultiCall.multicall()

All tests passing (945 tests).

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Apply check-effects-interactions pattern to:
- CrowdFund.unpledge()
- CustomError.withdraw()
- Payable.withdraw() and withdrawTo()
- SendingEther.sendViaSend()
- TokenLocker.withdraw()
- UniswapV3Swap.swapExactInputSingle()

All 945 tests passing.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/applications/MinimalProxy.sol 0.00% 1 Missing ⚠️
src/basic/Call.sol 75.00% 1 Missing ⚠️
src/basic/Payable.sol 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@KaelSensei
KaelSensei merged commit 9571edd into main Apr 6, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants