Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces direct-access guards to the implementation contract by adding an immutable Changes
Sequence DiagramsequenceDiagram
participant Client
participant Proxy
participant Impl as Implementation
rect rgba(100, 150, 200, 0.5)
Note over Client,Impl: Direct Call (Blocked)
Client->>Impl: call unknown selector
Impl->>Impl: check __self == address(this)
Impl-->>Client: revert ImplementationDeniedDirectAccess
end
rect rgba(100, 200, 150, 0.5)
Note over Client,Impl: Proxy Call (Allowed)
Client->>Proxy: call unknown selector
Proxy->>Impl: delegatecall (__self != address(this))
Impl-->>Proxy: fallback processes call
Proxy-->>Client: return result
end
rect rgba(200, 100, 100, 0.5)
Note over Client,Impl: Direct ETH Transfer (Blocked)
Client->>Impl: send ETH
Impl->>Impl: check __self == address(this)
Impl-->>Client: revert ImplementationDeniedDirectAccess
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~13 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Unit Test Coverage ReportCoverage after merging worktree-fallback-blocker into master will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Slither reportStatic Analysis Report**THIS CHECKLIST IS NOT COMPLETE**. Use `--show-ignored-findings` to show all the results. Summary - [locked-ether](#locked-ether) (2 results) (Medium) - [reentrancy-no-eth](#reentrancy-no-eth) (3 results) (Medium) - [unused-return](#unused-return) (9 results) (Medium) - [incorrect-modifier](#incorrect-modifier) (1 results) (Low) - [calls-loop](#calls-loop) (2 results) (Low) - [reentrancy-benign](#reentrancy-benign) (3 results) (Low) - [reentrancy-events](#reentrancy-events) (4 results) (Low) - [dead-code](#dead-code) (44 results) (Informational) - [solc-version](#solc-version) (3 results) (Informational) - [missing-inheritance](#missing-inheritance) (2 results) (Informational) - [naming-convention](#naming-convention) (14 results) (Informational) - [unimplemented-functions](#unimplemented-functions) (4 results) (Informational) - [unindexed-event-address](#unindexed-event-address) (1 results) (Informational) ## locked-ether Impact: Medium Confidence: High - [ ] ID-0 Contract locking ether found: Contract [Implementation](https://github.com/equilibria-xyz/root/blob/375b2632c93ec88ee4105c2889c1bbe496003e9e/src/mutability/Implementation.sol#L13-L90) has payable functions: - [Implementation.fallback()](https://github.com/equilibria-xyz/root/blob/375b2632c93ec88ee4105c2889c1bbe496003e9e/src/mutability/Implementation.sol#L82-L84) - [Implementation.receive()](https://github.com/equilibria-xyz/root/blob/375b2632c93ec88ee4105c2889c1bbe496003e9e/src/mutability/Implementation.sol#L87-L89) But does not have a function to withdraw the etherroot/src/mutability/Implementation.sol Lines 13 to 90 in 375b263
reentrancy-no-ethImpact: Medium
root/src/mutability/Mutator.sol Lines 34 to 46 in 375b263
root/src/mutability/Mutable.sol Lines 123 to 127 in 375b263
root/src/mutability/Mutable.sol Lines 97 to 113 in 375b263 unused-returnImpact: Medium
root/src/token/types/Token6.sol Lines 43 to 45 in 375b263
root/src/distribution/Airdrop.sol Lines 46 to 51 in 375b263
root/src/token/types/Token.sol Lines 51 to 53 in 375b263
root/src/token/types/Token18.sol Lines 53 to 55 in 375b263
root/src/token/types/Token6.sol Lines 54 to 56 in 375b263
root/src/distribution/Airdrop.sol Lines 28 to 34 in 375b263
root/src/token/types/Token.sol Lines 40 to 42 in 375b263
root/src/mutability/Mutator.sol Lines 34 to 46 in 375b263
root/src/token/types/Token18.sol Lines 43 to 45 in 375b263 incorrect-modifierImpact: Low
root/src/attribute/Attribute.sol Lines 28 to 32 in 375b263 calls-loopImpact: Low
root/src/mutability/Mutator.sol Lines 62 to 65 in 375b263
root/src/mutability/Mutator.sol Lines 57 to 60 in 375b263 reentrancy-benignImpact: Low
root/src/mutability/Mutator.sol Lines 57 to 60 in 375b263
root/src/mutability/Mutator.sol Lines 62 to 65 in 375b263
root/src/mutability/Mutator.sol Lines 34 to 46 in 375b263 reentrancy-eventsImpact: Low
root/src/mutability/Mutable.sol Lines 116 to 120 in 375b263
root/src/mutability/Mutator.sol Lines 62 to 65 in 375b263
root/src/mutability/Mutable.sol Lines 123 to 127 in 375b263
root/src/mutability/Mutator.sol Lines 57 to 60 in 375b263 dead-codeImpact: Informational
root/src/number/types/UFixed18.sol Lines 288 to 290 in 375b263
root/src/number/types/Fixed6.sol Lines 295 to 297 in 375b263
root/src/number/types/UFixed6.sol Lines 295 to 297 in 375b263
root/src/number/types/Fixed6.sol Lines 287 to 289 in 375b263
root/src/mutability/Implementation.sol Lines 69 to 71 in 375b263
root/src/number/types/Fixed18.sol Lines 288 to 290 in 375b263
root/src/number/types/Fixed6.sol Lines 311 to 313 in 375b263
root/src/number/types/Fixed6.sol Lines 327 to 329 in 375b263
root/src/number/types/UFixed6.sol Lines 311 to 314 in 375b263
root/src/number/types/Fixed6.sol Lines 335 to 337 in 375b263
root/src/number/types/Fixed18.sol Lines 304 to 306 in 375b263
root/src/number/types/Fixed6.sol Lines 343 to 346 in 375b263
root/src/number/types/UFixed18.sol Lines 313 to 316 in 375b263
root/src/number/types/UFixed18.sol Lines 280 to 282 in 375b263
root/src/number/types/UFixed6.sol Lines 329 to 331 in 375b263
root/src/number/types/Fixed18.sol Lines 280 to 282 in 375b263
root/src/number/types/Fixed6.sol Lines 303 to 305 in 375b263
root/src/number/types/Fixed6.sol Lines 361 to 363 in 375b263
root/src/number/types/UFixed6.sol Lines 279 to 281 in 375b263
root/src/number/types/Fixed6.sol Lines 369 to 371 in 375b263
root/src/number/types/UFixed18.sol Lines 330 to 332 in 375b263
root/src/number/types/UFixed18.sol Lines 272 to 274 in 375b263
root/src/number/types/Fixed6.sol Lines 352 to 355 in 375b263
root/src/number/types/Fixed6.sol Lines 319 to 321 in 375b263
root/src/number/types/UFixed18.sol Lines 296 to 298 in 375b263
root/src/mutability/Implementation.sol Lines 74 to 76 in 375b263
root/src/number/types/Fixed18.sol Lines 312 to 314 in 375b263
root/src/number/types/UFixed6.sol Lines 320 to 323 in 375b263
root/src/number/types/Fixed18.sol Lines 345 to 348 in 375b263
root/src/number/types/Fixed18.sol Lines 336 to 339 in 375b263
root/src/number/types/Fixed18.sol Lines 362 to 364 in 375b263
root/src/number/types/Fixed18.sol Lines 320 to 322 in 375b263
root/src/number/types/UFixed18.sol Lines 264 to 266 in 375b263
root/src/number/types/UFixed6.sol Lines 303 to 305 in 375b263
root/src/number/types/UFixed18.sol Lines 304 to 307 in 375b263
root/src/number/types/UFixed18.sol Lines 322 to 324 in 375b263
root/src/number/types/UFixed6.sol Lines 271 to 273 in 375b263
root/src/number/types/UFixed6.sol Lines 263 to 265 in 375b263
root/src/number/types/Fixed18.sol Lines 296 to 298 in 375b263
root/src/number/types/Fixed18.sol Lines 354 to 356 in 375b263
root/src/number/types/UFixed6.sol Lines 337 to 339 in 375b263
root/src/number/types/UFixed6.sol Lines 287 to 289 in 375b263
root/src/number/types/UFixed18.sol Lines 256 to 258 in 375b263
root/src/number/types/Fixed18.sol Lines 328 to 330 in 375b263 solc-versionImpact: Informational
root/src/attribute/Attribute.sol Line 2 in 375b263
root/src/number/types/Fixed18.sol Line 2 in 375b263
root/src/vrgda/VRGDADecayMath.sol Line 2 in 375b263 missing-inheritanceImpact: Informational
root/src/utils/OwnableStub.sol Lines 9 to 15 in 375b263
naming-conventionImpact: Informational
root/src/attribute/Pausable.sol Lines 34 to 36 in 375b263
root/src/mutability/Mutable.sol Lines 37 to 41 in 375b263
root/src/mutability/Mutable.sol Line 34 in 375b263
root/src/attribute/Ownable.sol Line 21 in 375b263
root/src/attribute/Ownable.sol Lines 42 to 44 in 375b263
root/src/mutability/Implementation.sol Line 31 in 375b263
root/src/mutability/Implementation.sol Line 21 in 375b263
root/src/attribute/Ownable.sol Lines 24 to 28 in 375b263
root/src/mutability/Implementation.sol Lines 24 to 28 in 375b263
root/src/attribute/Pausable.sol Line 23 in 375b263
root/src/attribute/Pausable.sol Lines 26 to 30 in 375b263
root/src/mutability/Implementation.sol Line 79 in 375b263
root/src/attribute/Attribute.sol Lines 20 to 24 in 375b263
root/src/attribute/Attribute.sol Line 17 in 375b263 unimplemented-functionsImpact: Informational
root/src/attribute/Withdrawable.sol Lines 11 to 18 in 375b263
root/src/mutability/Implementation.sol Lines 13 to 90 in 375b263
root/src/attribute/Delegatable.sol Lines 12 to 20 in 375b263
root/src/attribute/Executable.sol Lines 12 to 21 in 375b263 unindexed-event-addressImpact: Informational
|
Adds
fallback()andreceive()toImplementationthat revert when called directly (not via proxydelegatecall), preventing ETH from getting stuck in the implementation and blocking undefined-selector calls on the implementation's storage.Uses the
address(this) == __selfimmutable pattern from OZ'sUUPSUpgradeable._checkProxy().Note
Medium Risk
Changes core upgradeable
Implementationbehavior by introducing newfallback/receiverevert paths, which could affect integrations that previously (intentionally or accidentally) interacted with implementation addresses. Proxy behavior should be unaffected, but the guard must be correct to avoid breaking delegatecall usage.Overview
Prevents direct interaction with upgradeable implementation contracts by adding
fallback()andreceive()guards toImplementationthat revert with the newImplementationDeniedDirectAccesserror when not executing behind a proxy.Updates tests to cover the new direct-access protection (direct unknown-selector calls and ETH transfers now revert, while the proxy path remains allowed) and adjusts several test casts to
payable(address(...))to satisfy Solidity’s address-payable requirements.Written by Cursor Bugbot for commit 1394122. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Tests