Security: Fix critical vulnerabilities in AMM program#5
Open
ExpertVagabond wants to merge 1 commit into0xNineteen:mainfrom
Open
Security: Fix critical vulnerabilities in AMM program#5ExpertVagabond wants to merge 1 commit into0xNineteen:mainfrom
ExpertVagabond wants to merge 1 commit into0xNineteen:mainfrom
Conversation
Fixes found during security audit: 1. [CRITICAL] swap.rs:83 - Copy-paste bug: vault_dst constraint validated vault_src.mint instead of vault_dst.mint, allowing wrong-mint vault substitution 2. [HIGH] liquidity.rs:45-46 - Integer truncation in exchange rate calculation. Division before multiplication caused precision loss that could be exploited to extract value from the pool. Fixed by using u128 intermediate and multiplying first. 3. [HIGH] init_pool.rs - No fee parameter validation. Anyone could initialize a pool with fee_denominator=0 (causing permanent DoS via division-by-zero) or fee_numerator > fee_denominator. Added InvalidFees error and validation. 4. [MEDIUM] liquidity.rs:41 - Unchecked addition overflow in initial LP mint calculation. Solana compiles release mode with overflow-checks=false. Fixed with checked_add. 5. [MEDIUM] liquidity.rs:69,165 - Unchecked arithmetic on total_amount_minted tracking. Fixed with checked_add/checked_sub. Full audit report: https://gist.github.com/ExpertVagabond/security-audit-anchor-uniswap-v2 Co-Authored-By: Claude Opus 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.
Security Audit Report: anchor-uniswap-v2
Summary
Automated security audit identified 10 vulnerabilities (1 Critical, 4 High, 5 Medium). This PR fixes the 5 most impactful issues.
Vulnerabilities Fixed
1. [CRITICAL] Copy-Paste Bug in Swap Vault Constraint (swap.rs:83)
vault_dstconstraint checkedvault_src.mint == user_src.mintinstead ofvault_dst.mint == user_dst.mint. Combined with missing PDA validation on swap vaults, this could allow vault substitution attacks to drain pool funds.2. [HIGH] Integer Truncation in Exchange Rate (liquidity.rs:45-46)
Division before multiplication (
vault_balance1 / vault_balance0 * amount_liq0) caused severe precision loss. When vault ratio isn't a whole number, depositors could deposit less token1 than required, extracting value from existing LPs. Fixed with u128 intermediate:amount_liq0 * vault_balance1 / vault_balance0.3. [HIGH] No Fee Parameter Validation (init_pool.rs)
initialize_poolaccepted arbitrary fee parameters. Settingfee_denominator = 0causes permanent DoS (division-by-zero in every swap). Settingfee_numerator > fee_denominatorcauses underflow. Since pool_state is a unique PDA per mint pair, this permanently bricks the pool.4. [MEDIUM] Unchecked Overflow in Initial LP Mint (liquidity.rs:41)
amount_liq0 + amount_liq1uses unchecked addition. Solana compiles withoverflow-checks = falsein release mode, so overflow wraps silently instead of panicking.5. [MEDIUM] Unchecked Arithmetic on total_amount_minted (liquidity.rs:69,165)
+=and-=ontotal_amount_minteduse unchecked operators. In Solana's release mode, overflow/underflow wraps silently. This value is critical to pool accounting.Additional Issues Found (Not Fixed in This PR)
Verification
All fixes use safe Rust patterns:
checked_add/checked_sub/checked_mul/checked_divfor all arithmeticAudit Methodology
Full line-by-line code review of all Rust program files:
programs/ammv2/src/instructions/swap.rsprograms/ammv2/src/instructions/liquidity.rsprograms/ammv2/src/instructions/init_pool.rsprograms/ammv2/src/state.rsprograms/ammv2/src/error.rsAudited by Claude Code (Opus 4.6) for Superteam Earn Agent Bounty.