fix: patch 8 security vulnerabilities in escrow and fundraiser programs#17
Open
Potdealer wants to merge 1 commit intoeacc-labs:mainfrom
Open
fix: patch 8 security vulnerabilities in escrow and fundraiser programs#17Potdealer wants to merge 1 commit intoeacc-labs:mainfrom
Potdealer wants to merge 1 commit intoeacc-labs:mainfrom
Conversation
…ograms Escrow (Anchor): - Add missing deposit() call in make instruction (tokens never transferred to vault) - Fix maker_ata_y mint constraint from mint_x to mint_y - Fix wrong mint account in take deposit transfer CPI - Fix PDA seed endianness mismatch (to_be_bytes -> to_le_bytes) in withdraw and close - Fix vault closure rent destination from taker to maker (add maker account to Take struct) Fundraiser (Pinocchio): - Fix inverted target comparison in checker (>= to <) that allowed premature withdrawal - Add error propagation (?) on invoke_signed CPI in checker - Add contributor PDA validation in refund to prevent unauthorized claims Full audit report in SECURITY-AUDIT.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
@Potdealer can you please provide the test cases results. |
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
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
Security audit identifying and fixing 8 vulnerabilities (3 Critical, 4 High, 1 Medium) across the Anchor escrow and Pinocchio fundraiser programs.
Escrow Program (Anchor) — 5 findings
deposit()call inmakeinstruction — vault is created empty, escrow is non-functionalmaker_ata_yinstead ofmint_y) passed totransfer_checkedCPI intake— runtime failuremaker_ata_y— validated againstmint_xinstead ofmint_yto_be_bytes()inwithdraw/closevsto_le_bytes()used at PDA creation, causing signer verification failureFundraiser Program (Pinocchio) — 3 findings
checker— allows fund withdrawal before target is met, blocks withdrawal after target is met?oninvoke_signedCPI inchecker— transfer errors silently discardedrefund— any signer can claim another contributor's refundFiles Changed
anchor/escrow/programs/escrow/src/lib.rs— Added deposit callanchor/escrow/programs/escrow/src/instructions/make.rs— Fixed mint constraintanchor/escrow/programs/escrow/src/instructions/take.rs— Fixed mint in CPI, endianness, close destination, added maker accountpinocchio/fundraiser/src/instructions/checker.rs— Fixed comparison, added error propagationpinocchio/fundraiser/src/instructions/refund.rs— Added contributor PDA validationFull detailed audit report with severity ratings, impact analysis, and proof of concept for each finding is included in
SECURITY-AUDIT.md.Test plan
makeinstruction deposits tokens into vaulttakeinstruction completes full exchange lifecyclecheckeronly allows withdrawal when target is metrefundrejects contributor accounts not derived from signer🤖 Generated with Claude Code