Multiliquid: Solana Swap Program

Description

When a U64DynamicAddress type is processed in the get_nav function, it reads the nav from the first account in the unchecked remaining_accounts array. There are a few issues with how this is done.

1. Only the first of the user-provided remaining_accounts is ever read from and used for all dynamic prices of the asset.
2. The only requirement for this account is that it be owned by a specified nav_program_id account. Should this nav_program_id own multiple accounts then any one of them could be passed and read as a price for any other assets in this process.

Recommendation

1. Either iterate through the remaining_accounts or don't allow multiple dynamic pricing accounts for a single asset
2. Determine if supporting dynamic accounts is required and whether one could not ask the data account owner for the price of the asset rather than directly querying the data account. Another possibility could include having the pricing data account owned by the swap program itself and tracking which account would go to which asset.

Description

There is no access control on the global config initialization. A malicious actor can frontrun the initialization with their own parameters. If this goes undetected, user funds could be at risk.

Recommendation

Consider one of the following:

1. Restrict initialization to privileged accounts.
2. Always ensure initialization scripts validate configurations after the transaction submission.

Description

The update_asset_config_account() function allows the protocol admin to modify the asset_type field of an AssetConfig account after initialization. This will break the swap implementation, causing swaps to fail.

The swap() function enforces strict asset type validation at swap.rs:

// check that the asset types are correctrequire!(ctx.accounts.rwa_config.asset_type == AssetType::Rwa, ErrorCode::InvalidAssetType);require!(ctx.accounts.stable_config.asset_type == AssetType::Stable, ErrorCode::InvalidAssetType);

When an admin changes the asset_type of an asset from its original designation (e.g., changing a Stable to an Rwa, or vice versa), all swaps involving that asset will permanently revert with an InvalidAssetType error. This effectively bricks all liquidity pools using that asset. However, LPs are not affected, as such validations do not happen in the withdrawal liquidity-related functions.

Recommendation

Make asset_type immutable after initialization by removing it from the update_asset_config_account() function:

pub fn update_asset_config_account(      ctx: Context<UpdateAssetConfigAccount>,      nav_data: Vec<NavData>,      price_difference_bps: u16,      // Remove asset_type parameter  ) -> Result<()> {      ....      // Remove: asset_config.asset_type = asset_type;
      Ok(())  }

If asset type updates must be supported for legitimate reasons, add comprehensive validation:

1. Check that no active pairs exist using this asset before allowing type changes
2. Add a time-lock mechanism requiring a delay before the change takes effect
3. Emit events to warn LPs of the upcoming change
4. Consider adding a separate "migration" function with stricter controls

Description

The protocol validates individual fee parameters but fails to validate that combined fees (pair fees + protocol fees) don't exceed 100%, causing transaction panics during swaps.

1. init_pair / update_pair

require!(redemption_fee_bps <= 9900, ErrorCode::OutOfRange);  // Up to 99%require!(discount_rate_bps <= 9900, ErrorCode::OutOfRange);    // Up to 99%

2. init_global_config

require!(protocol_fees_bps <= 9900, ErrorCode::OutOfRange);  // Up to 99%

As a result, the total fees calculated exceed the amount_in parameter during stable-to-asset swaps, resulting in an underflow.

PoC

Consider the following scenario:

1. Admin sets protocol_fees_bps = 200 (2%)
2. LP sets redemption_fee_bps = 9900 (99%)
3. User attempts swap: 9900 + 200 = 10100 > 10000
4. Transaction panics with a MathOverflow error instead of meaningful message

Recommendation

Add validations in all fee configuration functions to ensure the total_fee_bps remains less than 10,000. Or consider adding a validation in the calculate_swap_results() function.

Description

The update_global_config() function allows updating protocol_fees_bps without any validation, unlike init_global_config(), which enforces a maximum of 9900 BPS.

As a result, the function allows setting protocol_fees_bps to any u16 value (up to 65535), breaking protocol_fee_bps assumptions throughout the protocol.

Recommendation

Consider adding the same validation used in init_global_config():

if let Some(protocol_fees_bps) = protocol_fees_bps {+   require!(protocol_fees_bps <= 9900, ErrorCode::OutOfRange);    new_protocol_fees_bps = protocol_fees_bps;}

Description

The close_pair instruction fails to close the SPL token vault accounts (stable_coin_vault_token_account and asset_token_vault_token_account) when the reference counter (used) reaches zero.

While the instruction properly:

1. Transfers remaining tokens back to the LP
2. Closes the UserVaultInfo PDA accounts

It does not close the actual SPL token vault accounts themselves, resulting in orphaned token accounts that permanently lock rent on-chain.

Recommendation

Add SPL token account closure to the close_pair() instruction when used == 0:

close_account(  CpiContext::new_with_signer(     ctx.accounts.token_program_stable.to_account_info(),        CloseAccount {          account: ctx.accounts.stable_coin_vault_token_account.to_account_info(),          destination: ctx.accounts.admin.to_account_info(),          authority: ctx.accounts.program_authority.to_account_info(),        },       &[signer_seeds],   ))?;

Description

The swap() function in swap.rs lacks slippage protection mechanisms, exposing users to risk from price volatility, front-running attacks, and MEV exploitation. The function accepts only an amount parameter without any slippage tolerance check. Users cannot specify the minimum acceptable amount of tokens they expect to receive.

Recommendation

Consider implementing comprehensive slippage protection by adding limit parameters to the swap() function.

Multiliquid

Fixed in 1f8e467 and c134b5a

Cantina

The fixes look accurate, account for all possible scenarios, and allow users to protect their amountIn and amountOut. However, these two parameters are optional but should be provided to avoid MEV attacks.

Description

The codebase contains several instances of dead code that is defined but never used throughout the program:

1. Unused Macro The assert_range macro is defined and exported with #[macro_export] but has zero usages across the entire codebase.

2. Unused Error Code Variants Seven error code enum variants are defined, but never referenced in any instruction or validation logic:

• InvalidNewAdmin
• NotAdmin
• InvalidMintAddress
• MustProvidePriceDecimals
• MustProvideNavProgramIdOrNavAccountDiscriminator
• MustProvideNavPriceOffset
• LpFeeMustBePositive

Recommendation

1. Remove the unused assert_range macro from macro.rs
2. Remove all seven unused error code variants from error.rs to maintain a clean error enumeration that only includes actively used error cases.

Description

The spl_helpers::transfer() function accepts generic AccountInfo parameters for the from and to accounts instead of strongly-typed InterfaceAccount<'info, TokenAccount> parameters.

Current state:

• All nine usage instances (swap.rs, claim_fees.rs, close_pair.rs, add_liquidity.rs, remove_liquidity.rs) properly validate token accounts at the instruction level with InterfaceAccount<'info, TokenAccount> and appropriate constraints.
• Accounts are downgraded to AccountInfo when passed to the helper via .to_account_info()
• The helper relies entirely on the SPL Token Program validation during CPI.

While the current implementation is secure, it weakens Rust's type-safety guarantees. Future code modifications or new call sites could accidentally pass incorrect account types, with errors only caught at runtime during CPI execution rather than at compile-time or during Anchor validation.

Recommendation

1. Change the helper function signature to enforce strong typing and remove downgrading to type AccountInfo while passing it to the helper in all usage instances.

from: &InterfaceAccount<'info, TokenAccount>,to: &InterfaceAccount<'info, TokenAccount>

2. Alternatively, add relevant documentation in the spl_helpers::transfer function to ensure this behavior is properly documented for future developers.

Description

The AssetConfig struct contains a version field that is defined but never used anywhere in the codebase. The version field is documented as "version of the NavData struct".

Recommendation

Consider removing the unused field (or) implement version tracking if required and start from version 1 instead of 0.

Description

The math.rs module inconsistently uses error codes for arithmetic operations. Multiple instances of checked_sub() (subtraction) incorrectly return MathOverflow instead of MathUnderflow, making debugging significantly harder.

Recommendation

Update all checked_sub() operations to use MathUnderflow error code.

Description

- /// NavData is used to store info about the methpds to retrieve the NAV data for a specific asset+ /// NavData is used to store info about the methods to retrieve the NAV data for a specific asset
- // DRY function to read the price from an account (expecting the price to be stored as au64)+ // DRY function to read the price from an account (expecting the price to be stored as u64)

Description

The inline documentation for the NavData enum in asset_config.rs states that "Currently we support three methods" for retrieving NAV data. However, the actual implementation supports four distinct methods. The fourth variant, PythPush, is completely omitted from the documentation comment.

Recommendation

Update the inline documentation to reflect all four supported NAV data retrieval methods accurately.

Description

The get_nav() function in asset_config.rs performs a price difference validation check that is vulnerable to arithmetic overflow when NAV values are significant (has less price decimals).

When max_nav is sufficiently large (e.g., 1e18 = 1e9 with zero decimals), the multiplication operation max_nav.checked_mul(10000 - price_difference_bps as u64) overflows the u64 type and returns None. The subsequent .unwrap() call then panics with the error Option::unwrap() on a None value.

This issue results in:

1. Denial of Service: Any asset with a NAV above approximately 1.84 × 10^15 (with 9 decimals) will cause all transactions to fail.
2. Unpredictable Failures: The issue only manifests when NAV reaches certain thresholds, making it difficult to diagnose.
3. No Graceful Error Handling: The transaction panics rather than returning a proper error code

PoC

• max_nav = 1_000_000_000_000_000_000 (1 quintillion, representing a price of 1e9 asset with 0 decimals) - price_difference_bps = 500 (5% tolerance)
• Calculation: 1_000_000_000_000_000_000 × (10000 - 500) = 1_000_000_000_000_000_000 × 9500 - Result: 9.5 × 10^21, which exceeds u64::MAX (18,446,744,073,709,551,615 ≈ 1.84 × 10^19) - The checked_mul() returns None, causing a panic

Recommendation

1. Replace all .unwrap() calls with proper error handling using .ok_or(ErrorCode::MathOverflow)? which will return meaningful error.

2. Add comprehensive tests with edge cases: Maximum safe NAV values, Minimum price_difference_bps values, and Various combinations that approach u64 limits.

3. Document maximum supported NAV values in the code comments.

Multiliquid

Fixed by 4cf5446b3adfa8d12d8dc7f86a7007d6f3ffbec5 (for now, we've added error handling with MathOverflow)

The likelihood is really close to 0 (most of the stables will have a value around $1, and for RWAs, we can't imagine having such a NAV. Since it is not something a random user (the NAV) could set, we don't see a security issue here either.

Cantina

While the newly added error checks will detect and revert on overflows with more explicit error messages, the protocol’s capacity to safely handle large NAV values remains constrained. It may still result in unintended or unpredictable behavior.

Description

The set_new_admin() function lacks validation to ensure that the new admin address differs from the current admin address. This allows the current admin to unnecessarily set themselves as the pending admin again, wasting transaction fees and emitting misleading events without actual state changes.

Recommendation

Add validation to prevent setting the new admin to the current admin address:

require!(new_admin != global_config.admin, ErrorCode::AlreadyAdmin);

Description

The swap instruction uses u64 to represent token amounts, which significantly restricts the maximum swappable amount for tokens with high decimal precision.

For tokens with 18 decimals, the maximum value of u64 (18,446,744,073,709,551,615) translates to only approximately 18.45 tokens in human-readable terms:

u64::MAX / 10^18 = 18,446,744,073,709,551,615 / 1,000,000,000,000,000,000 ≈ 18.45

This limitation severely restricts the protocol's usability for:

• High-value swaps involving tokens with 18 decimals
• Liquidity provision operations that require larger amounts
• Any operation where users need to transact more than ~18 tokens

The issue affects not just swap operations but potentially all instruction parameters that handle token amounts using u64, including add_liquidity(), remove_liquidity(), and related functions.

Recommendation

Replace u64 with u128 for all token amount parameters throughout the program. The u128 type provides sufficient range to handle tokens with up to 38 decimals at scale:

u128::MAX / 10^18 ≈ 340 trillion tokens

Description

The update_asset_config_account() function contains unnecessary computation where mint_address is assigned or validated.

Since the asset_config account is a PDA derived with mint_address as a seed, the Anchor framework already enforces that the account can only be accessed with the correct mint_address used in derivation.

Any assignment or validation of this field is redundant and wastes compute units.

Recommendation

Remove the redundant mint_address assignment or validation from the update_asset_config_account function.

Description

In the init_pair() and init_asset_config_account() functions, the pause state is explicitly set to false, which is already the default value for boolean fields in newly initialized accounts. This redundant assignment wastes compute units without providing any functional benefit.

Recommendation

Remove the explicit pause state assignment to false in both init_pair() and init_asset_config_account() functions.

Multiliquid

Fixed in e8625298 and 7f3591ab

Description

Multiple instructions across the entire program use AccountInfo instead of UncheckedAccount, against the anchor's suggestion.

Recommendation

Consider replacing all instances of AccountInfo in-favor of UncheckedAccount

Multiliquid

Fixed in c19af01 and c134b5a

Description

The amount_in_for_vault.checked_add(protocol_fees).unwrap() could panic if the addition overflows.

Recommendation

Consider using .ok_or at the end of the checked_add to ensure the overflow is caught and reverted accordingly.

Description

The swap() function in swap.rs makes CPI calls to transfer protocol fees even when the fee amount is zero, unnecessarily wasting compute units.

Recommendation

Consider adding a conditional check to skip the transfer CPI when protocol fees are zero (only if protocol_fees can be zero). If it will never be set to zero, then adding an additional check is redundant and this finding could be safely "acknowledged".