Conditional Validation Bypass Remediation

Overview

Related Detector: Conditional Validation Bypass

Conditional validation bypass occurs when security checks are placed inside conditional branches, allowing attackers to reach critical operations through unvalidated execution paths. The fix is to move all security validations above any conditional logic so they execute unconditionally.

Recommended Fix

Before (Vulnerable)

pub fn process_action(
    accounts: &[AccountInfo],
    action: u8,
    amount: u64,
) -> ProgramResult {
    let authority = &accounts[0];
    let target = &accounts[1];

    match action {
        0 => {
            // Read action -- no validation needed (correct)
            let data = target.try_borrow_data()?;
            msg!("Balance: {}", u64::from_le_bytes(data[0..8].try_into().unwrap()));
        }
        1 => {
            // Write action -- validation inside branch
            if !authority.is_signer {
                return Err(ProgramError::MissingRequiredSignature);
            }
            let mut data = target.try_borrow_mut_data()?;
            data[0..8].copy_from_slice(&amount.to_le_bytes());
        }
        _ => {
            // VULNERABLE: no validation on unknown action types
            let mut data = target.try_borrow_mut_data()?;
            data[0..8].copy_from_slice(&0u64.to_le_bytes());
        }
    }
    Ok(())
}

After (Fixed)

pub fn process_action(
    accounts: &[AccountInfo],
    action: u8,
    amount: u64,
) -> ProgramResult {
    let authority = &accounts[0];
    let target = &accounts[1];

    // FIXED: separate read and write into different functions
    // Or validate on all mutation paths:
    if action != 0 {
        // All non-read actions require signer
        if !authority.is_signer {
            return Err(ProgramError::MissingRequiredSignature);
        }
    }

    match action {
        0 => {
            let data = target.try_borrow_data()?;
            msg!("Balance: {}", u64::from_le_bytes(data[0..8].try_into().unwrap()));
        }
        1 => {
            let mut data = target.try_borrow_mut_data()?;
            data[0..8].copy_from_slice(&amount.to_le_bytes());
        }
        _ => {
            return Err(ProgramError::InvalidArgument); // Reject unknown actions
        }
    }
    Ok(())
}

Alternative Mitigations

1. Separate functions for different privilege levels

Instead of branching within one function, use separate instruction handlers:

pub fn process_instruction(
    program_id: &Pubkey,
    accounts: &[AccountInfo],
    data: &[u8],
) -> ProgramResult {
    match data[0] {
        0 => process_read(accounts),         // No auth needed
        1 => process_write(accounts, data),   // Auth required
        _ => Err(ProgramError::InvalidInstructionData),
    }
}

fn process_write(accounts: &[AccountInfo], data: &[u8]) -> ProgramResult {
    // Signer check is unconditional within this function
    if !accounts[0].is_signer {
        return Err(ProgramError::MissingRequiredSignature);
    }
    // ... state modification
    Ok(())
}

2. Anchor instruction separation

Anchor naturally prevents this pattern by separating instructions into distinct handlers with typed account validation:

#[program]
pub mod my_program {
    pub fn read_balance(ctx: Context<ReadBalance>) -> Result<()> {
        // No Signer in context -- read-only
        Ok(())
    }

    pub fn update_balance(ctx: Context<UpdateBalance>, amount: u64) -> Result<()> {
        // Signer is enforced by an account-validation framework constraints
        ctx.accounts.target.balance = amount;
        Ok(())
    }
}

#[derive(Accounts)]
pub struct UpdateBalance<'info> {
    pub authority: Signer<'info>,  // Unconditional signer validation
    #[account(mut, has_one = authority)]
    pub target: Account<'info, MyState>,
}

3. Guard-first pattern

Place all security checks at the top of the function before any branching:

pub fn process(accounts: &[AccountInfo], data: &[u8]) -> ProgramResult {
    // All validations first, unconditionally
    let authority = &accounts[0];
    if !authority.is_signer {
        return Err(ProgramError::MissingRequiredSignature);
    }

    let target = &accounts[1];
    if target.owner != program_id {
        return Err(ProgramError::IllegalOwner);
    }

    // Now branch on instruction type -- all paths are validated
    match data[0] {
        0 => handle_deposit(accounts, data),
        1 => handle_withdraw(accounts, data),
        _ => Err(ProgramError::InvalidInstructionData),
    }
}

Common Mistakes

Mistake 1: Validating only the “expected” branch

if expected_path {
    verify_signer(accounts)?;
    do_operation(accounts)?;
} else {
    // Developer assumes this path won't be taken
    // but attacker controls the condition
    do_operation(accounts)?;
}

Mistake 2: Using a user-supplied flag to skip validation

// WRONG: attacker sets skip_validation = true
pub fn process(accounts: &[AccountInfo], skip_validation: bool) -> ProgramResult {
    if !skip_validation {
        check_authority(accounts)?;
    }
    modify_state(accounts)?;
    Ok(())
}

Never use caller-supplied parameters to conditionally enable or disable security checks.

Mistake 3: Early return before validation on error paths

pub fn process(accounts: &[AccountInfo], amount: u64) -> ProgramResult {
    if amount == 0 {
        // Returns early, but still modifies state before returning
        let mut data = accounts[1].try_borrow_mut_data()?;
        data[0] = 0; // State modified without validation
        return Ok(());
    }
    // Validation only on non-zero path
    if !accounts[0].is_signer { return Err(...); }
    // ...
}