Vault Manipulation Remediation

Overview

Related Detector: Vault Manipulation

Vault manipulation vulnerabilities allow attackers to modify vault parameters (collateral ratios, fees, reserve balances) without authorization. The fix requires enforcing access control on every vault state write and validating that parameter values fall within safe bounds.

Recommended Fix

Before (Vulnerable)

pub fn update_vault_config(
    accounts: &[AccountInfo],
    new_ratio: u64,
    new_fee: u64,
) -> ProgramResult {
    let vault = &accounts[0];
    // No access control -- anyone can modify
    let mut data = vault.try_borrow_mut_data()?;
    let mut config = VaultConfig::try_from_slice(&data[8..])?;
    config.collateral_ratio = new_ratio;
    config.fee_bps = new_fee;
    config.serialize(&mut &mut data[8..])?;
    Ok(())
}

After (Fixed)

pub fn update_vault_config(
    accounts: &[AccountInfo],
    new_ratio: u64,
    new_fee: u64,
) -> ProgramResult {
    let vault = &accounts[0];
    let admin = &accounts[1];

    // FIXED: verify admin signer
    if !admin.is_signer {
        return Err(ProgramError::MissingRequiredSignature);
    }

    let data = vault.try_borrow_data()?;
    let config = VaultConfig::try_from_slice(&data[8..])?;

    // FIXED: verify admin key matches vault admin
    if config.admin != *admin.key {
        return Err(ProgramError::InvalidAccountData);
    }

    // FIXED: enforce parameter bounds
    if new_ratio < MIN_COLLATERAL_RATIO || new_ratio > MAX_COLLATERAL_RATIO {
        return Err(ProgramError::InvalidArgument);
    }
    if new_fee > MAX_FEE_BPS {
        return Err(ProgramError::InvalidArgument);
    }

    drop(data);
    let mut data = vault.try_borrow_mut_data()?;
    let mut config = VaultConfig::try_from_slice(&data[8..])?;
    config.collateral_ratio = new_ratio;
    config.fee_bps = new_fee;
    config.serialize(&mut &mut data[8..])?;
    Ok(())
}

Alternative Mitigations

1. PDA-derived vault authority

Use a PDA as the vault authority so that only the program itself can modify vault state:

let (vault_authority, bump) = Pubkey::find_program_address(
    &[b"vault", vault.key.as_ref()],
    program_id,
);
// Vault writes are only possible through program logic
// since only the program can produce the PDA signer seeds

2. an account-validation framework constraints for vault operations

#[derive(Accounts)]
pub struct UpdateVault<'info> {
    #[account(
        mut,
        has_one = admin,
        constraint = vault.collateral_ratio >= MIN_RATIO
    )]
    pub vault: Account<'info, VaultState>,
    pub admin: Signer<'info>,
}

3. Timelock for parameter changes

For high-impact parameter changes, add a timelock so changes are delayed and can be reviewed:

pub fn propose_ratio_change(ctx: Context<ProposeChange>, new_ratio: u64) -> Result<()> {
    let proposal = &mut ctx.accounts.proposal;
    proposal.new_value = new_ratio;
    proposal.execute_after = Clock::get()?.unix_timestamp + TIMELOCK_SECONDS;
    Ok(())
}

pub fn execute_ratio_change(ctx: Context<ExecuteChange>) -> Result<()> {
    let proposal = &ctx.accounts.proposal;
    let clock = Clock::get()?;
    require!(clock.unix_timestamp >= proposal.execute_after, ErrorCode::TimelockActive);
    // Apply change
    Ok(())
}

Common Mistakes

Mistake 1: Checking signer but not matching against vault admin

if !accounts[1].is_signer {
    return Err(ProgramError::MissingRequiredSignature);
}
// WRONG: any signer passes -- must verify key matches vault.admin

Mistake 2: Missing bounds validation on parameters

// Signer is validated, but ratio has no bounds check
vault_state.collateral_ratio = user_supplied_ratio; // Could be 0

Even authorized admins should be constrained by protocol-safe bounds.

Mistake 3: Validating in a non-dominating code path

if some_condition {
    if !admin.is_signer { return Err(...); }
}
// Attacker takes the else branch -- no validation
vault_state.ratio = new_ratio;

Validation must occur on every execution path that reaches the state modification.