Skip to content

Commit 3121721

Browse files
authored
fix(lang): guard AccountLoader against buffer truncation (#4633)
1 parent f76b01a commit 3121721

12 files changed

Lines changed: 381 additions & 2 deletions

File tree

.github/workflows/reusable-tests.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,8 @@ jobs:
458458
path: tests/pyth
459459
- cmd: cd tests/realloc && anchor test --skip-lint && npx tsc --noEmit
460460
path: tests/realloc
461+
- cmd: cd tests/accountloader-realloc && anchor test --skip-lint && npx tsc --noEmit
462+
path: tests/accountloader-realloc
461463
- cmd: cd tests/system-accounts && anchor test --skip-lint
462464
path: tests/system-accounts
463465
- cmd: cd tests/misc && anchor test --skip-lint && npx tsc --noEmit

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ The minor version will be incremented upon a breaking change and the patch versi
3131

3232
### Fixes
3333

34+
- lang: Guard `AccountLoader<T>::exit` against zero-copy buffer truncation and bail with `AccountDidNotDeserialize` instead of rewriting the discriminator over an undersized buffer ([#4633](https://github.com/otter-sec/anchor/pull/4633)).
3435
- lang: Set `anchor-lang` Minimum Supported Rust Version to `1.89` ([#4638](https://github.com/otter-sec/anchor/pull/4638)).
3536
- lang: Shorten invariant lifetimes during `Context` creation ([#4363](https://github.com/solana-foundation/anchor/pull/4363)).
3637
- ts: Guard recursive IDL layouts against stack overflows while preserving supported recursive types ([#4604](https://github.com/solana-foundation/anchor/pull/4604)).

lang/src/accounts/account_loader.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,11 @@ impl<'info, T: ZeroCopy + Owner> AccountsExit<'info> for AccountLoader<'info, T>
268268
fn exit(&self, program_id: &Pubkey) -> Result<()> {
269269
// Only persist if the owner is the current program and the account is not closed.
270270
if &T::owner() == program_id && !crate::common::is_closed(self.acc_info) {
271+
// Guard against truncation: refuse to rewrite the discriminator over an undersized buffer.
272+
let required = T::DISCRIMINATOR.len() + mem::size_of::<T>();
273+
if self.acc_info.try_data_len()? < required {
274+
return Err(ErrorCode::AccountDidNotDeserialize.into());
275+
}
271276
let mut data = self.acc_info.try_borrow_mut_data()?;
272277
let dst: &mut [u8] = &mut data;
273278
let mut writer = BpfWriter::new(dst);

lang/tests/account_loader_truncation.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ macro_rules! setup_truncated_account {
3333
#[test]
3434
fn test_load_truncated() {
3535
setup_truncated_account!(key, owner, lamports, data, account_info);
36-
let loader: AccountLoader<ZcStruct> = AccountLoader::try_from(&account_info).unwrap();
36+
let loader: AccountLoader<ZcStruct> =
37+
AccountLoader::try_from_unchecked(&crate::ID, &account_info).unwrap();
3738
assert_eq!(
3839
loader.load().unwrap_err(),
3940
ErrorCode::AccountDidNotDeserialize.into()
@@ -43,7 +44,8 @@ fn test_load_truncated() {
4344
#[test]
4445
fn test_load_mut_truncated() {
4546
setup_truncated_account!(key, owner, lamports, data, account_info);
46-
let loader: AccountLoader<ZcStruct> = AccountLoader::try_from(&account_info).unwrap();
47+
let loader: AccountLoader<ZcStruct> =
48+
AccountLoader::try_from_unchecked(&crate::ID, &account_info).unwrap();
4749
assert_eq!(
4850
loader.load_mut().unwrap_err(),
4951
ErrorCode::AccountDidNotDeserialize.into()
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[programs.localnet]
2+
accountloader_realloc = "8GM8KqKaxYb1jEbn5TiqqPqJYsChhjYvfpT2f6KTmUKb"
3+
4+
[provider]
5+
cluster = "localnet"
6+
wallet = "~/.config/solana/id.json"
7+
8+
[scripts]
9+
test = "yarn run ts-mocha -p ./tsconfig.json -t 1000000 tests/**/*.ts"
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[workspace]
2+
members = [
3+
"programs/*"
4+
]
5+
resolver = "2"
6+
7+
[profile.release]
8+
overflow-checks = true
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"name": "accountloader-realloc",
3+
"version": "0.1.0",
4+
"license": "(MIT OR Apache-2.0)",
5+
"homepage": "https://github.com/otter-sec/anchor#readme",
6+
"bugs": {
7+
"url": "https://github.com/otter-sec/anchor/issues"
8+
},
9+
"repository": {
10+
"type": "git",
11+
"url": "https://github.com/otter-sec/anchor.git"
12+
},
13+
"engines": {
14+
"node": ">=17"
15+
},
16+
"scripts": {
17+
"test": "anchor test"
18+
}
19+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
[package]
2+
name = "accountloader-realloc"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
[lib]
7+
crate-type = ["cdylib", "lib"]
8+
name = "accountloader_realloc"
9+
10+
[features]
11+
no-entrypoint = []
12+
no-log-ix-name = []
13+
cpi = ["no-entrypoint"]
14+
default = []
15+
idl-build = ["anchor-lang/idl-build"]
16+
17+
[profile.release]
18+
overflow-checks = true
19+
20+
[dependencies]
21+
anchor-lang = { path = "../../../../lang" }
22+
bytemuck = { version = "1.4.0", features = ["derive", "min_const_generics"] }
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
use anchor_lang::prelude::*;
2+
3+
declare_id!("8GM8KqKaxYb1jEbn5TiqqPqJYsChhjYvfpT2f6KTmUKb");
4+
5+
#[program]
6+
pub mod accountloader_realloc {
7+
use super::*;
8+
9+
/// Create the zero-copy account with the full required footprint
10+
/// (`DISCRIMINATOR.len() + size_of::<Data>()`).
11+
pub fn initialize(ctx: Context<Initialize>) -> Result<()> {
12+
let mut data = ctx.accounts.data.load_init()?;
13+
data.value = 42;
14+
Ok(())
15+
}
16+
17+
/// Shrink the `AccountLoader` to `new_len` bytes
18+
pub fn shrink(_ctx: Context<Shrink>, _new_len: u16) -> Result<()> {
19+
Ok(())
20+
}
21+
22+
/// Verify the account stays readable after the shrink.
23+
pub fn read(ctx: Context<Read>) -> Result<u64> {
24+
let data = ctx.accounts.data.load()?;
25+
Ok(data.value)
26+
}
27+
28+
/// Creates an account as an older program version left it: v1 footprint
29+
/// with the current discriminator. (In a real upgrade the struct name —
30+
/// and therefore the discriminator — doesn't change; the V1/V2 split
31+
/// exists only so both layouts can coexist in this test.)
32+
pub fn initialize_legacy(ctx: Context<InitializeLegacy>) -> Result<()> {
33+
let mut data = ctx.accounts.counter.try_borrow_mut_data()?;
34+
data[..8].copy_from_slice(CounterV2::DISCRIMINATOR);
35+
let v1 = CounterV1 { value: 42 };
36+
data[8..].copy_from_slice(bytemuck::bytes_of(&v1));
37+
Ok(())
38+
}
39+
40+
/// Grows the legacy account to the v2 footprint via the `realloc`
41+
/// constraint, then fills the new field from existing v1 data.
42+
pub fn migrate(ctx: Context<Migrate>) -> Result<()> {
43+
let mut counter = ctx.accounts.counter.load_mut()?;
44+
counter.extra = counter.value * 2;
45+
Ok(())
46+
}
47+
48+
pub fn read_extra(ctx: Context<ReadCounter>) -> Result<u64> {
49+
Ok(ctx.accounts.counter.load()?.extra)
50+
}
51+
}
52+
53+
#[derive(Accounts)]
54+
pub struct Initialize<'info> {
55+
#[account(mut)]
56+
pub authority: Signer<'info>,
57+
58+
#[account(
59+
init,
60+
payer = authority,
61+
seeds = [b"data"],
62+
bump,
63+
space = 8 + core::mem::size_of::<Data>(),
64+
)]
65+
pub data: AccountLoader<'info, Data>,
66+
67+
pub system_program: Program<'info, System>,
68+
}
69+
70+
#[derive(Accounts)]
71+
#[instruction(new_len: u16)]
72+
pub struct Shrink<'info> {
73+
#[account(mut)]
74+
pub authority: Signer<'info>,
75+
#[account(
76+
mut,
77+
seeds = [b"data"],
78+
bump,
79+
realloc = new_len as usize,
80+
realloc::payer = authority,
81+
realloc::zero = false,
82+
)]
83+
pub data: AccountLoader<'info, Data>,
84+
85+
pub system_program: Program<'info, System>,
86+
}
87+
88+
#[derive(Accounts)]
89+
pub struct Read<'info> {
90+
#[account(seeds = [b"data"], bump)]
91+
pub data: AccountLoader<'info, Data>,
92+
}
93+
94+
#[derive(Accounts)]
95+
pub struct InitializeLegacy<'info> {
96+
#[account(mut)]
97+
pub authority: Signer<'info>,
98+
99+
/// CHECK: holds the v1 layout; written manually in the handler.
100+
#[account(
101+
init,
102+
payer = authority,
103+
seeds = [b"legacy"],
104+
bump,
105+
space = 8 + core::mem::size_of::<CounterV1>(),
106+
owner = crate::ID,
107+
)]
108+
pub counter: UncheckedAccount<'info>,
109+
110+
pub system_program: Program<'info, System>,
111+
}
112+
113+
#[derive(Accounts)]
114+
pub struct Migrate<'info> {
115+
#[account(mut)]
116+
pub authority: Signer<'info>,
117+
118+
#[account(
119+
mut,
120+
seeds = [b"legacy"],
121+
bump,
122+
realloc = 8 + core::mem::size_of::<CounterV2>(),
123+
realloc::payer = authority,
124+
realloc::zero = false,
125+
)]
126+
pub counter: AccountLoader<'info, CounterV2>,
127+
128+
pub system_program: Program<'info, System>,
129+
}
130+
131+
#[derive(Accounts)]
132+
pub struct ReadCounter<'info> {
133+
#[account(seeds = [b"legacy"], bump)]
134+
pub counter: AccountLoader<'info, CounterV2>,
135+
}
136+
137+
#[account(zero_copy)]
138+
#[repr(C)]
139+
pub struct Data {
140+
pub value: u64,
141+
pub padding: [u8; 64],
142+
}
143+
144+
/// The original account layout (8-bytes body + 8-bytes discriminator = 16 bytes).
145+
#[zero_copy]
146+
pub struct CounterV1 {
147+
pub value: u64,
148+
}
149+
150+
/// The upgraded layout: `extra` was added. (16 bytes body + 8 bytes discriminator = 24 bytes)
151+
#[account(zero_copy)]
152+
#[repr(C)]
153+
pub struct CounterV2 {
154+
pub value: u64,
155+
pub extra: u64,
156+
}

0 commit comments

Comments
 (0)