Add clarifying comments for common security audit false positives#588
Open
joshuahannan wants to merge 1 commit intomasterfrom
Open
Add clarifying comments for common security audit false positives#588joshuahannan wants to merge 1 commit intomasterfrom
joshuahannan wants to merge 1 commit intomasterfrom
Conversation
Documents the intent and safety of patterns that are frequently misidentified as vulnerabilities by automated analysis tools: - FlowToken.cdc: Service account address exclusions are a documented performance optimization, not a security bypass - FlowClusterQC.cdc: voteThreshold() is an inclusive lower bound so >= in isComplete() is correct; clarify the 2/3 quorum math - FlowTransactionScheduler.cdc: slotUsedEffort addition cannot overflow due to upstream validation; getStatus() inference only applies to aged-out transactions (Scheduled txs are always in the transactions map); setConfig() is admin-only via UpdateConfig entitlement - FlowIDTableStaking.cdc: delegatorIDs are scoped per node — (nodeID, delegatorID) is the unique key; empty vault in addNodeRecord is intentional, tokens are validated and committed immediately after - LockedTokens.cdc: unlockLimit precondition prevents underflow - get_total_balance.cdc: mark script as broken/outdated (getAuthAccount is not available in Cadence 1.0 scripts) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
During a security audit, several code patterns were identified that are consistently misidentified as vulnerabilities by automated tools and AI-assisted bug bounty reporters. This PR adds inline comments at each location explaining the intent and why the pattern is safe, to reduce noise from invalid bug reports.
Patterns documented:
FlowToken.cdc— Service account address exclusions inTokensWithdrawn/TokensDepositedare a documented performance optimization for epoch transitions, not a security bypassFlowClusterQC.cdc—voteThreshold()returns an inclusive lower bound, so>=inisComplete()is correct; the threshold itself enforces strictly >2/3 quorumFlowTransactionScheduler.cdc—slotUsedEffortaddition is bounded by upstream validation and cannot overflow;getStatus()inference logic only applies to aged-out transactions (Scheduled txs are always in the active map and returned earlier);setConfig()is admin-only viaUpdateConfigentitlementFlowIDTableStaking.cdc—delegatorIDCounteris scoped per node so(nodeID, delegatorID)is the unique key; the empty vault inaddNodeRecordis intentional —tokensCommittedis validated against the minimum and deposited immediately afterLockedTokens.cdc—unlockLimitprecondition prevents underflow inwithdrawUnlockedTokensget_total_balance.cdc— Marked as outdated/broken:getAuthAccountis not available in Cadence 1.0 scripts and this template cannot be used on mainnetTest plan
make cipasses (all Cadence + Go tests, check-tidy)🤖 Generated with Claude Code