Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions lang/src/accounts/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,13 @@ impl<'info, T: ZeroCopy + Owner> AccountLoader<'info, T> {
return Err(ErrorCode::AccountDiscriminatorMismatch.into());
}

let end = required_zero_copy_len::<T>(disc.len())?;
if data.len() < end {
return Err(ErrorCode::AccountDidNotDeserialize.into());
}

Ok(Ref::map(data, |data| {
bytemuck::from_bytes(&data[disc.len()..mem::size_of::<T>() + disc.len()])
bytemuck::from_bytes(&data[disc.len()..end])
}))
}

Expand All @@ -189,10 +194,13 @@ impl<'info, T: ZeroCopy + Owner> AccountLoader<'info, T> {
return Err(ErrorCode::AccountDiscriminatorMismatch.into());
}

let end = required_zero_copy_len::<T>(disc.len())?;
if data.len() < end {
return Err(ErrorCode::AccountDidNotDeserialize.into());
}

Ok(RefMut::map(data, |data| {
bytemuck::from_bytes_mut(
&mut data.deref_mut()[disc.len()..mem::size_of::<T>() + disc.len()],
)
bytemuck::from_bytes_mut(&mut data.deref_mut()[disc.len()..end])
}))
}

Expand All @@ -209,20 +217,35 @@ impl<'info, T: ZeroCopy + Owner> AccountLoader<'info, T> {

// The discriminator should be zero, since we're initializing.
let disc = T::DISCRIMINATOR;
if data.len() < disc.len() {
return Err(ErrorCode::AccountDiscriminatorNotFound.into());
}
let given_disc = &data[..disc.len()];
let has_disc = given_disc.iter().any(|b| *b != 0);
if has_disc {
return Err(ErrorCode::AccountDiscriminatorAlreadySet.into());
}

let end = required_zero_copy_len::<T>(disc.len())?;
if data.len() < end {
return Err(ErrorCode::AccountDidNotDeserialize.into());
}

Ok(RefMut::map(data, |data| {
bytemuck::from_bytes_mut(
&mut data.deref_mut()[disc.len()..mem::size_of::<T>() + disc.len()],
)
bytemuck::from_bytes_mut(&mut data.deref_mut()[disc.len()..end])
}))
}
}

/// Returns `disc_len + size_of::<T>()`, mapping arithmetic overflow to a
/// structured `AccountDidNotDeserialize` error rather than panicking.
#[inline]
fn required_zero_copy_len<T>(disc_len: usize) -> Result<usize> {
disc_len
.checked_add(mem::size_of::<T>())
.ok_or_else(|| ErrorCode::AccountDidNotDeserialize.into())
}

impl<'info, B, T: ZeroCopy + Owner> Accounts<'info, B> for AccountLoader<'info, T> {
#[inline(never)]
fn try_accounts(
Expand Down
7 changes: 7 additions & 0 deletions lang/syn/src/codegen/accounts/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,13 @@ pub fn generate_constraint_zeroed(
quote! {
let #field: #ty_decl = {
let mut __data: &[u8] = &#field.try_borrow_data()?;
if __data.len() < #discriminator.len() {
return Err(
anchor_lang::error::Error::from(
anchor_lang::error::ErrorCode::AccountDiscriminatorNotFound
).with_account_name(#name_str)
);
}
let __disc = &__data[..#discriminator.len()];
let __has_disc = __disc.iter().any(|b| *b != 0);
if __has_disc {
Expand Down
145 changes: 145 additions & 0 deletions lang/tests/account_loader_truncation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
//! Regression tests for `AccountLoader::{load, load_mut, load_init}` panicking
//! on accounts that pass the discriminator length check but whose data is
//! truncated before the end of the zero-copy body.
//!
//! Before the fix, the three accessors sliced
//! data[disc.len()..disc.len() + size_of::<T>()]
//! without verifying the upper bound, which caused an index-out-of-bounds
//! panic (surfaced to clients as `Program failed to complete`) instead of a
//! structured `AnchorError`. `load_init` additionally panicked on the
//! discriminator slice itself when the account was shorter than the
//! discriminator.
//!
//! Tracking: solana-foundation/anchor#4509

use {
anchor_lang::{error::ErrorCode, prelude::*},
std::mem,
};

declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");

#[account(zero_copy)]
#[derive(Default, Debug)]
pub struct Foo {
pub a: u64,
pub b: u64,
pub c: u64,
}

/// Build an `AccountInfo` whose data is exactly `len` bytes long. If
/// `with_disc` is true the discriminator prefix is written; otherwise the
/// buffer is zeroed.
fn make_owned_account_bytes(len: usize, with_disc: bool) -> Vec<u8> {
let mut data = vec![0u8; len];
if with_disc {
let disc = Foo::DISCRIMINATOR;
let n = std::cmp::min(disc.len(), len);
data[..n].copy_from_slice(&disc[..n]);
}
data
}

#[test]
fn load_returns_error_when_data_shorter_than_zero_copy_body() {
// Discriminator-sized but truncated body — used to panic.
let mut data = make_owned_account_bytes(Foo::DISCRIMINATOR.len(), true);
let mut lamports: u64 = 1;
let owner: Pubkey = crate::ID;
let key: Pubkey = Pubkey::new_unique();

let acc_info: AccountInfo<'_> =
AccountInfo::new(&key, false, false, &mut lamports, &mut data, &owner, false);

let loader: AccountLoader<'_, Foo> = AccountLoader::try_from(&acc_info).unwrap();
let err = loader.load().unwrap_err();
assert_eq!(
err,
anchor_lang::error::Error::from(ErrorCode::AccountDidNotDeserialize)
);
}

#[test]
fn load_mut_returns_error_when_data_shorter_than_zero_copy_body() {
// One byte short of the full zero-copy body — used to panic.
let needed = Foo::DISCRIMINATOR.len() + mem::size_of::<Foo>();
let mut data = make_owned_account_bytes(needed - 1, true);
let mut lamports: u64 = 1;
let owner: Pubkey = crate::ID;
let key: Pubkey = Pubkey::new_unique();

let acc_info: AccountInfo<'_> =
AccountInfo::new(&key, false, true, &mut lamports, &mut data, &owner, false);

let loader: AccountLoader<'_, Foo> = AccountLoader::try_from(&acc_info).unwrap();
let err = loader.load_mut().unwrap_err();
assert_eq!(
err,
anchor_lang::error::Error::from(ErrorCode::AccountDidNotDeserialize)
);
}

#[test]
fn load_init_returns_error_when_data_shorter_than_zero_copy_body() {
// Discriminator-region is zero (as init expects) but body is missing.
let mut data = make_owned_account_bytes(Foo::DISCRIMINATOR.len(), false);
let mut lamports: u64 = 1;
let owner: Pubkey = crate::ID;
let key: Pubkey = Pubkey::new_unique();

let acc_info: AccountInfo<'_> =
AccountInfo::new(&key, false, true, &mut lamports, &mut data, &owner, false);

let loader: AccountLoader<'_, Foo> =
AccountLoader::try_from_unchecked(&crate::ID, &acc_info).unwrap();
let err = loader.load_init().unwrap_err();
assert_eq!(
err,
anchor_lang::error::Error::from(ErrorCode::AccountDidNotDeserialize)
);
}

#[test]
fn load_init_returns_error_when_data_shorter_than_discriminator() {
// Below `disc.len()` — `load_init` previously sliced without bounds-checking
// the discriminator prefix and panicked.
let mut data = vec![0u8; Foo::DISCRIMINATOR.len() - 1];
let mut lamports: u64 = 1;
let owner: Pubkey = crate::ID;
let key: Pubkey = Pubkey::new_unique();

let acc_info: AccountInfo<'_> =
AccountInfo::new(&key, false, true, &mut lamports, &mut data, &owner, false);

let loader: AccountLoader<'_, Foo> =
AccountLoader::try_from_unchecked(&crate::ID, &acc_info).unwrap();
let err = loader.load_init().unwrap_err();
assert_eq!(
err,
anchor_lang::error::Error::from(ErrorCode::AccountDiscriminatorNotFound)
);
}

#[test]
fn load_succeeds_on_exactly_sized_account() {
// Sanity: the happy path is unaffected by the new bound check.
let needed = Foo::DISCRIMINATOR.len() + mem::size_of::<Foo>();
let mut data = make_owned_account_bytes(needed, true);
let mut lamports: u64 = 1;
let owner: Pubkey = crate::ID;
let key: Pubkey = Pubkey::new_unique();

let acc_info: AccountInfo<'_> =
AccountInfo::new(&key, false, true, &mut lamports, &mut data, &owner, false);

let loader: AccountLoader<'_, Foo> = AccountLoader::try_from(&acc_info).unwrap();
{
let foo = loader.load().unwrap();
assert_eq!(foo.a, 0);
}
{
let mut foo = loader.load_mut().unwrap();
foo.a = 7;
}
assert_eq!(loader.load().unwrap().a, 7);
}
Loading