This repository was archived by the owner on Feb 5, 2026. It is now read-only.
Implement security fixes for Permitter contract #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Addresses critical and high severity findings from security audit: - C-01: Add cap validation in constructor to prevent owner DoS - H-01: Implement 1-hour timelock for cap/signer updates (schedule/execute pattern) - H-02: Validate newMaxTotalEth >= totalEthRaised in executeUpdateMaxTotalEth - H-03: Reject zero caps in constructor validation - M-01: Add authorizedCaller parameter and validation in validateBid Key changes: - Add UPDATE_DELAY constant (1 hour) for timelock protection - Replace instant update functions with schedule/execute pattern - Add authorizedCaller to Permitter constructor and factory - Add pending state variables for scheduled updates - Add new errors: InvalidCap, CapBelowCurrentAmount, UnauthorizedCaller, UpdateNotScheduled, UpdateTooEarly - Add new events: CapUpdateScheduled, SignerUpdateScheduled, AuthorizedCallerUpdated - Add updateAuthorizedCaller function (no timelock for emergency CCA changes) - Update all test files to use new interface All 92 tests pass including unit, integration, fuzz, and exploit verification tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| uint256 _maxTokensPerBidder, | ||
| address _owner | ||
| address _owner, | ||
| address _authorizedCaller |
Check notice
Code scanning / Slither
Missing zero address validation Low
Comment on lines
+163
to
+180
| function executeUpdateMaxTotalEth() external onlyOwner { | ||
| if (pendingMaxTotalEthTime == 0) revert UpdateNotScheduled(); | ||
| if (block.timestamp < pendingMaxTotalEthTime) { | ||
| revert UpdateTooEarly(pendingMaxTotalEthTime, block.timestamp); | ||
| } | ||
| if (pendingMaxTotalEth < totalEthRaised) { | ||
| revert CapBelowCurrentAmount(pendingMaxTotalEth, totalEthRaised); | ||
| } | ||
|
|
||
| uint256 oldCap = maxTotalEth; | ||
| maxTotalEth = newMaxTotalEth; | ||
| emit CapUpdated(CapType.TOTAL_ETH, oldCap, newMaxTotalEth); | ||
| maxTotalEth = pendingMaxTotalEth; | ||
|
|
||
| // Clear pending update | ||
| pendingMaxTotalEth = 0; | ||
| pendingMaxTotalEthTime = 0; | ||
|
|
||
| emit CapUpdated(CapType.TOTAL_ETH, oldCap, maxTotalEth); | ||
| } |
Check notice
Code scanning / Slither
Block timestamp Low
Comment on lines
+222
to
+236
| function executeUpdateTrustedSigner() external onlyOwner { | ||
| if (pendingTrustedSignerTime == 0) revert UpdateNotScheduled(); | ||
| if (block.timestamp < pendingTrustedSignerTime) { | ||
| revert UpdateTooEarly(pendingTrustedSignerTime, block.timestamp); | ||
| } | ||
|
|
||
| address oldSigner = trustedSigner; | ||
| trustedSigner = newSigner; | ||
| emit SignerUpdated(oldSigner, newSigner); | ||
| trustedSigner = pendingTrustedSigner; | ||
|
|
||
| // Clear pending update | ||
| pendingTrustedSigner = address(0); | ||
| pendingTrustedSignerTime = 0; | ||
|
|
||
| emit SignerUpdated(oldSigner, trustedSigner); | ||
| } |
Check notice
Code scanning / Slither
Block timestamp Low
| } | ||
|
|
||
| /// @inheritdoc IPermitter | ||
| function updateAuthorizedCaller(address newCaller) external onlyOwner { |
Check notice
Code scanning / Slither
Missing zero address validation Low
- Rename test functions in ExploitTests.t.sol from testFix_/testRemaining_ to test_ prefix to satisfy scopelint naming convention - Add test_RevertIf_ExceedsGlobalMaxTokensPerBidder to achieve 100% line coverage for Permitter.sol (covers line 131) - Apply scopelint formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Coverage after merging rsolari/skill-inquiry into main will be
Coverage Report
|
|||||||||||||||||||||||||
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements critical and high-severity security fixes identified in the Permitter contract audit:
Key Changes
schedule*()+execute*()functionsTest Coverage
🤖 Generated with Claude Code