From 37d18580bac0d07c856f06321068814921e6ade5 Mon Sep 17 00:00:00 2001 From: acheron Date: Fri, 19 Dec 2025 20:38:18 +0100 Subject: [PATCH 1/8] tests: Add basic security checks tests for `InterfaceAccount` --- tests/interface-account/Anchor.toml | 11 +++ tests/interface-account/Cargo.toml | 14 +++ tests/interface-account/package.json | 16 ++++ .../programs/interface-account/Cargo.toml | 21 +++++ .../programs/interface-account/src/lib.rs | 52 +++++++++++ .../interface-account/programs/new/Cargo.toml | 18 ++++ .../interface-account/programs/new/src/lib.rs | 47 ++++++++++ .../interface-account/programs/old/Cargo.toml | 18 ++++ .../interface-account/programs/old/src/lib.rs | 28 ++++++ .../tests/interface-account.ts | 90 +++++++++++++++++++ tests/interface-account/tsconfig.json | 10 +++ tests/package.json | 2 +- 12 files changed, 326 insertions(+), 1 deletion(-) create mode 100644 tests/interface-account/Anchor.toml create mode 100644 tests/interface-account/Cargo.toml create mode 100644 tests/interface-account/package.json create mode 100644 tests/interface-account/programs/interface-account/Cargo.toml create mode 100644 tests/interface-account/programs/interface-account/src/lib.rs create mode 100644 tests/interface-account/programs/new/Cargo.toml create mode 100644 tests/interface-account/programs/new/src/lib.rs create mode 100644 tests/interface-account/programs/old/Cargo.toml create mode 100644 tests/interface-account/programs/old/src/lib.rs create mode 100644 tests/interface-account/tests/interface-account.ts create mode 100644 tests/interface-account/tsconfig.json diff --git a/tests/interface-account/Anchor.toml b/tests/interface-account/Anchor.toml new file mode 100644 index 0000000000..619f4cc271 --- /dev/null +++ b/tests/interface-account/Anchor.toml @@ -0,0 +1,11 @@ +[programs.localnet] +interface_account = "interfaceAccount111111111111111111111111111" +new = "New1111111111111111111111111111111111111111" +old = "oLd1111111111111111111111111111111111111111" + +[provider] +cluster = "localnet" +wallet = "~/.config/solana/id.json" + +[scripts] +test = "yarn run ts-mocha -p ./tsconfig.json -t 1000000 tests/**/*.ts" diff --git a/tests/interface-account/Cargo.toml b/tests/interface-account/Cargo.toml new file mode 100644 index 0000000000..f397704811 --- /dev/null +++ b/tests/interface-account/Cargo.toml @@ -0,0 +1,14 @@ +[workspace] +members = [ + "programs/*" +] +resolver = "2" + +[profile.release] +overflow-checks = true +lto = "fat" +codegen-units = 1 +[profile.release.build-override] +opt-level = 3 +incremental = false +codegen-units = 1 diff --git a/tests/interface-account/package.json b/tests/interface-account/package.json new file mode 100644 index 0000000000..5bb13482cb --- /dev/null +++ b/tests/interface-account/package.json @@ -0,0 +1,16 @@ +{ + "name": "interface-account", + "version": "0.32.1", + "license": "(MIT OR Apache-2.0)", + "homepage": "https://github.com/coral-xyz/anchor#readme", + "bugs": { + "url": "https://github.com/coral-xyz/anchor/issues" + }, + "repository": { + "type": "git", + "url": "https://github.com/coral-xyz/anchor.git" + }, + "engines": { + "node": ">=17" + } +} diff --git a/tests/interface-account/programs/interface-account/Cargo.toml b/tests/interface-account/programs/interface-account/Cargo.toml new file mode 100644 index 0000000000..5a777958a5 --- /dev/null +++ b/tests/interface-account/programs/interface-account/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "interface-account" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] +name = "interface_account" + +[features] +default = [] +cpi = ["no-entrypoint"] +no-entrypoint = [] +no-idl = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang" } +new = { path = "../new", features = ["cpi"] } +old = { path = "../old", features = ["cpi"] } diff --git a/tests/interface-account/programs/interface-account/src/lib.rs b/tests/interface-account/programs/interface-account/src/lib.rs new file mode 100644 index 0000000000..a8cef9cd54 --- /dev/null +++ b/tests/interface-account/programs/interface-account/src/lib.rs @@ -0,0 +1,52 @@ +#![allow(warnings)] + +use anchor_lang::prelude::*; + +declare_id!("interfaceAccount111111111111111111111111111"); + +#[program] +pub mod interface_account { + use super::*; + + pub fn test(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct Test<'info> { + pub expected_account: InterfaceAccount<'info, interface::ExpectedAccount>, +} + +mod interface { + #[derive(Clone)] + pub struct ExpectedAccount(new::ExpectedAccount); + + impl anchor_lang::AccountDeserialize for ExpectedAccount { + fn try_deserialize(buf: &mut &[u8]) -> anchor_lang::Result { + new::ExpectedAccount::try_deserialize(buf).map(Self) + } + + fn try_deserialize_unchecked(buf: &mut &[u8]) -> anchor_lang::Result { + new::ExpectedAccount::try_deserialize_unchecked(buf).map(Self) + } + } + + impl anchor_lang::AccountSerialize for ExpectedAccount {} + + impl anchor_lang::Owners for ExpectedAccount { + fn owners() -> &'static [anchor_lang::prelude::Pubkey] { + &[old::ID_CONST, new::ID_CONST] + } + } + + #[cfg(feature = "idl-build")] + mod idl_impls { + use super::ExpectedAccount; + + impl anchor_lang::IdlBuild for ExpectedAccount {} + impl anchor_lang::Discriminator for ExpectedAccount { + const DISCRIMINATOR: &'static [u8] = &[]; + } + } +} diff --git a/tests/interface-account/programs/new/Cargo.toml b/tests/interface-account/programs/new/Cargo.toml new file mode 100644 index 0000000000..9d007ca48f --- /dev/null +++ b/tests/interface-account/programs/new/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "new" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] + +[features] +default = [] +cpi = ["no-entrypoint"] +no-entrypoint = [] +no-idl = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang" } diff --git a/tests/interface-account/programs/new/src/lib.rs b/tests/interface-account/programs/new/src/lib.rs new file mode 100644 index 0000000000..7b8c28fe0d --- /dev/null +++ b/tests/interface-account/programs/new/src/lib.rs @@ -0,0 +1,47 @@ +#![allow(warnings)] + +use anchor_lang::prelude::*; + +declare_id!("New1111111111111111111111111111111111111111"); + +#[program] +pub mod new { + use super::*; + + pub fn init(ctx: Context) -> Result<()> { + Ok(()) + } + + pub fn init_another(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct Init<'info> { + #[account(mut)] + pub authority: Signer<'info>, + #[account(init, payer = authority, space = 40)] + pub expected_account: Account<'info, ExpectedAccount>, + pub system_program: Program<'info, System>, +} + +#[derive(Accounts)] +pub struct InitAnother<'info> { + #[account(mut)] + pub authority: Signer<'info>, + #[account(init, payer = authority, space = 72)] + pub another_account: Account<'info, AnotherAccount>, + pub system_program: Program<'info, System>, +} + +#[account] +pub struct ExpectedAccount { + pub data: Pubkey, +} + +#[account] +pub struct AnotherAccount { + pub a: Pubkey, + pub b: Pubkey, +} diff --git a/tests/interface-account/programs/old/Cargo.toml b/tests/interface-account/programs/old/Cargo.toml new file mode 100644 index 0000000000..43381ffb6e --- /dev/null +++ b/tests/interface-account/programs/old/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "old" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] + +[features] +default = [] +cpi = ["no-entrypoint"] +no-entrypoint = [] +no-idl = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang" } diff --git a/tests/interface-account/programs/old/src/lib.rs b/tests/interface-account/programs/old/src/lib.rs new file mode 100644 index 0000000000..1fded4cf19 --- /dev/null +++ b/tests/interface-account/programs/old/src/lib.rs @@ -0,0 +1,28 @@ +#![allow(warnings)] + +use anchor_lang::prelude::*; + +declare_id!("oLd1111111111111111111111111111111111111111"); + +#[program] +pub mod old { + use super::*; + + pub fn init(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct Init<'info> { + #[account(mut)] + pub authority: Signer<'info>, + #[account(init, payer = authority, space = 40)] + pub expected_account: Account<'info, ExpectedAccount>, + pub system_program: Program<'info, System>, +} + +#[account] +pub struct ExpectedAccount { + pub data: Pubkey, +} diff --git a/tests/interface-account/tests/interface-account.ts b/tests/interface-account/tests/interface-account.ts new file mode 100644 index 0000000000..25e8c8406b --- /dev/null +++ b/tests/interface-account/tests/interface-account.ts @@ -0,0 +1,90 @@ +import * as anchor from "@coral-xyz/anchor"; +import assert from "assert"; + +import type { InterfaceAccount } from "../target/types/interface_account"; +import type { New } from "../target/types/new"; +import type { Old } from "../target/types/old"; + +describe("interface-account", () => { + anchor.setProvider(anchor.AnchorProvider.env()); + const program: anchor.Program = + anchor.workspace.interfaceAccount; + const oldProgram: anchor.Program = anchor.workspace.old; + const newProgram: anchor.Program = anchor.workspace.new; + + const oldExpectedAccount = anchor.web3.Keypair.generate(); + const newExpectedAccount = anchor.web3.Keypair.generate(); + const anotherAccount = anchor.web3.Keypair.generate(); + + // Initialize accounts + before(async () => { + // Initialize the expected account of the old program + await oldProgram.methods + .init() + .accounts({ expectedAccount: oldExpectedAccount.publicKey }) + .signers([oldExpectedAccount]) + .rpc(); + + // Initialize the expected account of the new program + await newProgram.methods + .init() + .accounts({ expectedAccount: newExpectedAccount.publicKey }) + .signers([newExpectedAccount]) + .rpc(); + + // Initialize another account of the new program + await newProgram.methods + .initAnother() + .accounts({ anotherAccount: anotherAccount.publicKey }) + .signers([anotherAccount]) + .rpc(); + }); + + it("Allows old exptected accounts", async () => { + await program.methods + .test() + .accounts({ expectedAccount: oldExpectedAccount.publicKey }) + .rpc(); + }); + + it("Allows new exptected accounts", async () => { + await program.methods + .test() + .accounts({ expectedAccount: newExpectedAccount.publicKey }) + .rpc(); + }); + + it("Doesn't allow accounts owned by other programs", async () => { + try { + await program.methods + .test() + .accounts({ expectedAccount: program.provider.wallet!.publicKey }) + .rpc(); + + assert.fail("Allowed unexpected account substitution!"); + } catch (e) { + assert(e instanceof anchor.AnchorError); + assert.strictEqual( + e.error.errorCode.number, + anchor.LangErrorCode.AccountOwnedByWrongProgram + ); + } + }); + + it("Doesn't allow unexpected accounts owned by the expected programs", async () => { + try { + await program.methods + .test() + .accounts({ expectedAccount: anotherAccount.publicKey }) + .rpc(); + + assert.fail("Allowed unexpected account substitution!"); + } catch (e) { + assert(e instanceof anchor.AnchorError); + assert.strictEqual( + e.error.errorCode.number, + anchor.LangErrorCode.AccountDiscriminatorMismatch + ); + } + }); +}); diff --git a/tests/interface-account/tsconfig.json b/tests/interface-account/tsconfig.json new file mode 100644 index 0000000000..adfaf3bb84 --- /dev/null +++ b/tests/interface-account/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "lib": ["es2020"], + "module": "commonjs", + "target": "es6", + "esModuleInterop": true, + "strict": true, + "skipLibCheck": true + } +} diff --git a/tests/package.json b/tests/package.json index 7571a236b2..f5428d6cc2 100644 --- a/tests/package.json +++ b/tests/package.json @@ -25,7 +25,7 @@ "floats", "idl", "ido-pool", - "interface", + "interface-account", "lazy-account", "lockup", "misc", From 680d331dcb3607cb60d1049e8ec6097102e04ac5 Mon Sep 17 00:00:00 2001 From: acheron Date: Sat, 20 Dec 2025 00:23:04 +0100 Subject: [PATCH 2/8] ci: Add `interface-account` tests --- .github/workflows/reusable-tests.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/reusable-tests.yaml b/.github/workflows/reusable-tests.yaml index 5819267891..3cc8718eea 100644 --- a/.github/workflows/reusable-tests.yaml +++ b/.github/workflows/reusable-tests.yaml @@ -472,6 +472,8 @@ jobs: path: tests/bench - cmd: cd tests/idl && ./test.sh path: tests/idl + - cmd: cd tests/interface-account && anchor test + path: tests/interface-account - cmd: cd tests/lazy-account && anchor test --skip-lint path: tests/lazy-account - cmd: cd tests/test-instruction-validation && ./test.sh From 6cdb82b533551518348d86d25bed49bc51ae4688 Mon Sep 17 00:00:00 2001 From: acheron Date: Sat, 20 Dec 2025 01:00:40 +0100 Subject: [PATCH 3/8] lang: Fix `InterfaceAccount::try_from` --- lang/src/accounts/interface_account.rs | 27 +++++++++----------------- 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index d12f8cd537..dabde72c24 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -234,28 +234,19 @@ impl<'a, T: AccountSerialize + AccountDeserialize + Clone> InterfaceAccount<'a, } impl<'a, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> InterfaceAccount<'a, T> { - /// Deserializes the given `info` into a `InterfaceAccount`. - /// - /// This **does not** check an Anchor discriminator. It first validates - /// program ownership via `T::check_owner`, then deserializes using - /// `AccountDeserialize::try_deserialize_unchecked`. + /// Deserializes the given `info` into an `InterfaceAccount`. #[inline(never)] pub fn try_from(info: &'a AccountInfo<'a>) -> Result { - // `InterfaceAccount` targets foreign program accounts (e.g., SPL Token - // accounts) that do not have Anchor discriminators. Because of that, we - // intentionally skip the Anchor discriminator check here and instead: - // - // 1) Validate program ownership via `T::check_owner(info.owner)?` - // 2) Deserialize without a discriminator by delegating to - // `T::try_deserialize_unchecked` - Self::try_from_unchecked(info) + if info.owner == &system_program::ID && info.lamports() == 0 { + return Err(ErrorCode::AccountNotInitialized.into()); + } + T::check_owner(info.owner)?; + let mut data: &[u8] = &info.try_borrow_data()?; + Ok(Self::new(info, T::try_deserialize(&mut data)?)) } - /// Deserializes the given `info` into a `InterfaceAccount` **without** checking - /// the account discriminator. This is intended for foreign program accounts. - /// Prefer `Self::try_from` when you also want the ownership check, but note - /// that both skip Anchor discriminator checks, and `try_from` additionally - /// enforces ownership. + /// Deserializes the given `info` into an `InterfaceAccount` without checking the account + /// discriminator. Be careful when using this and avoid it if possible. #[inline(never)] pub fn try_from_unchecked(info: &'a AccountInfo<'a>) -> Result { if info.owner == &system_program::ID && info.lamports() == 0 { From b650cb0c587f5e3c35f46bb101912af4779f96d3 Mon Sep 17 00:00:00 2001 From: acheron Date: Sun, 28 Dec 2025 01:31:01 +0100 Subject: [PATCH 4/8] lang: Fix `InterfaceAccount::reload` --- lang/src/accounts/interface_account.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index dabde72c24..f1699a750a 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -185,13 +185,8 @@ impl<'a, T: AccountSerialize + AccountDeserialize + Clone> InterfaceAccount<'a, /// Reloads the account from storage. This is useful, for example, when /// observing side effects after CPI. /// - /// No Anchor discriminator is checked during reload. Instead, this method enforces - /// owner stability by verifying that `info.owner == self.owner` (i.e., the pubkey - /// validated at construction) to avoid TOCTOU issues, and then re-deserializes `T` - /// from the updated bytes. - /// - /// If you need discriminator validation on reload, use `Account` with an Anchor - /// #[account] type. + /// This method also re-validates that the program owner has not changed + /// since the initial validation. pub fn reload(&mut self) -> Result<()> { let info = self.account.to_account_info(); @@ -203,7 +198,7 @@ impl<'a, T: AccountSerialize + AccountDeserialize + Clone> InterfaceAccount<'a, // Re-deserialize fresh data into the inner account. let mut data: &[u8] = &info.try_borrow_data()?; - let new_val = T::try_deserialize_unchecked(&mut data)?; + let new_val = T::try_deserialize(&mut data)?; self.account.set_inner(new_val); Ok(()) } From 0a0d279eff54370e910d2c95793bd7bd2cbd6831 Mon Sep 17 00:00:00 2001 From: acheron Date: Sun, 28 Dec 2025 11:10:38 +0100 Subject: [PATCH 5/8] lang: Allow multiple owners in `InterfaceAccount::reload` --- lang/src/accounts/interface_account.rs | 36 +++++++++++--------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index f1699a750a..a3ba29132d 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -182,27 +182,6 @@ impl<'a, T: AccountSerialize + AccountDeserialize + Clone> InterfaceAccount<'a, } } - /// Reloads the account from storage. This is useful, for example, when - /// observing side effects after CPI. - /// - /// This method also re-validates that the program owner has not changed - /// since the initial validation. - pub fn reload(&mut self) -> Result<()> { - let info = self.account.to_account_info(); - - // Enforce owner stability: must match the one validated at construction. - if info.owner != &self.owner { - return Err(Error::from(ErrorCode::AccountOwnedByWrongProgram) - .with_pubkeys((*info.owner, self.owner))); - } - - // Re-deserialize fresh data into the inner account. - let mut data: &[u8] = &info.try_borrow_data()?; - let new_val = T::try_deserialize(&mut data)?; - self.account.set_inner(new_val); - Ok(()) - } - pub fn into_inner(self) -> T { self.account.into_inner() } @@ -251,6 +230,21 @@ impl<'a, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> Interfac let mut data: &[u8] = &info.try_borrow_data()?; Ok(Self::new(info, T::try_deserialize_unchecked(&mut data)?)) } + + /// Reloads the account from storage. This is useful, for example, when observing side effects + /// after CPI. + /// + /// This method also validates that the account is owned by one of the expected programs. + pub fn reload(&mut self) -> Result<()> { + let info = self.account.to_account_info(); + T::check_owner(info.owner)?; + + // Re-deserialize fresh data into the inner account. + let mut data: &[u8] = &info.try_borrow_data()?; + let new_val = T::try_deserialize(&mut data)?; + self.account.set_inner(new_val); + Ok(()) + } } impl<'info, B, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> Accounts<'info, B> From 5be6bcacedcb31059ae0a7a7976a99fffd376b8c Mon Sep 17 00:00:00 2001 From: acheron Date: Sun, 28 Dec 2025 11:11:52 +0100 Subject: [PATCH 6/8] lang: Remove the unnecessary clone to get the account info --- lang/src/accounts/interface_account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index a3ba29132d..3da15b5d8b 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -236,7 +236,7 @@ impl<'a, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> Interfac /// /// This method also validates that the account is owned by one of the expected programs. pub fn reload(&mut self) -> Result<()> { - let info = self.account.to_account_info(); + let info: &AccountInfo = self.account.as_ref(); T::check_owner(info.owner)?; // Re-deserialize fresh data into the inner account. From fb62fc63c6912884eb59d698ecdbf4f5a579df9a Mon Sep 17 00:00:00 2001 From: acheron Date: Sun, 28 Dec 2025 11:21:58 +0100 Subject: [PATCH 7/8] lang: Fix compile error about mutable borrow --- lang/src/accounts/interface_account.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index 3da15b5d8b..f26b4e89d3 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -240,9 +240,10 @@ impl<'a, T: AccountSerialize + AccountDeserialize + CheckOwner + Clone> Interfac T::check_owner(info.owner)?; // Re-deserialize fresh data into the inner account. - let mut data: &[u8] = &info.try_borrow_data()?; - let new_val = T::try_deserialize(&mut data)?; - self.account.set_inner(new_val); + self.account.set_inner({ + let mut data: &[u8] = &info.try_borrow_data()?; + T::try_deserialize(&mut data)? + }); Ok(()) } } From 07cafaa4593b773ef14048bac1266d27bdafcbbe Mon Sep 17 00:00:00 2001 From: acheron Date: Sun, 28 Dec 2025 11:23:54 +0100 Subject: [PATCH 8/8] lang: Remove the unused imports --- lang/src/accounts/interface_account.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lang/src/accounts/interface_account.rs b/lang/src/accounts/interface_account.rs index f26b4e89d3..4665c36de8 100644 --- a/lang/src/accounts/interface_account.rs +++ b/lang/src/accounts/interface_account.rs @@ -1,14 +1,14 @@ //! Account container that checks ownership on deserialization. use crate::accounts::account::Account; -use crate::error::{Error, ErrorCode}; +use crate::error::ErrorCode; use crate::solana_program::account_info::AccountInfo; use crate::solana_program::instruction::AccountMeta; use crate::solana_program::pubkey::Pubkey; use crate::solana_program::system_program; use crate::{ AccountDeserialize, AccountSerialize, Accounts, AccountsClose, AccountsExit, CheckOwner, Key, - Owners, Result, ToAccountInfo, ToAccountInfos, ToAccountMetas, + Owners, Result, ToAccountInfos, ToAccountMetas, }; use std::collections::BTreeSet; use std::fmt;