Skip to content

fix: prevent panic on undersized zero-copy account deserialization (#4509)#4519

Open
Abhilashpatel12 wants to merge 14 commits into
otter-sec:masterfrom
Abhilashpatel12:master
Open

fix: prevent panic on undersized zero-copy account deserialization (#4509)#4519
Abhilashpatel12 wants to merge 14 commits into
otter-sec:masterfrom
Abhilashpatel12:master

Conversation

@Abhilashpatel12
Copy link
Copy Markdown

This PR fixes panic-triggerable zero-copy deserialization paths for under-sized accounts in AccountLoader::{load, load_mut, load_init} and generated #[account(zero)] validation code

@vercel
Copy link
Copy Markdown

vercel Bot commented May 11, 2026

@Abhilashpatel12 is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR prevents bytemuck-triggered panics in AccountLoader::{load, load_mut, load_init} and the generated #[account(zero)] validation code when an on-chain account is undersized. The fix adds a combined discriminator.len() + size_of::<T>() length guard before any unsafe byte-casting, and a narrower discriminator-only guard in the zero-constraint codegen path (full-size validation for that path is deferred to load_init()).

  • account_loader.rs: load(), load_mut(), and load_init() now return AccountDidNotDeserialize instead of panicking when data.len() < disc.len() + size_of::<T>(). A side-effect is that data shorter than the discriminator now also returns AccountDidNotDeserialize rather than the previously returned AccountDiscriminatorNotFound, which is a subtle observable error-code change.
  • constraints.rs: generate_constraint_zeroed gains a discriminator-length bounds check that prevents the out-of-bounds slice panic when account data is completely absent; the check variable __required holds only discriminator.len(), which is intentionally narrower than the full required size but the name is slightly misleading.
  • account_loader_truncation.rs: Regression tests confirm all three accessor methods return a structured error rather than panicking on truncated accounts, with a happy-path test for a correctly-sized account.

Confidence Score: 4/5

Safe to merge; all previously-panicking paths now return structured errors and are covered by new regression tests.

The core panic-prevention logic is correct and well-tested. The main concern is that accounts shorter than the discriminator will now surface AccountDidNotDeserialize from load() and load_mut() instead of AccountDiscriminatorNotFound, silently changing the error variant visible to any caller that matched on it.

lang/src/accounts/account_loader.rs — the error code returned for the sub-discriminator case changed and that change is not called out in the PR description or inline docs.

Important Files Changed

Filename Overview
lang/src/accounts/account_loader.rs Adds a combined size check (discriminator + size_of T) in load(), load_mut(), and load_init() to prevent bytemuck panics on undersized accounts; changes the error code for the sub-discriminator case from AccountDiscriminatorNotFound to AccountDidNotDeserialize.
lang/syn/src/codegen/accounts/constraints.rs Adds a bounds check in generate_constraint_zeroed() that guards against a panic when borrowing the discriminator slice on accounts shorter than the discriminator; the check variable __required is only discriminator length, not the full account size, which is intentional but the naming is slightly misleading.
lang/tests/account_loader_truncation.rs New regression test file covering load(), load_mut(), and load_init() with accounts that contain only discriminator bytes (no struct body), plus a happy-path test for a correctly-sized account.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["AccountLoader::load / load_mut / load_init"] --> B["try_borrow_data"]
    B --> C{"data.len() less than disc+size_of T?"}
    C -- YES --> D["Err: AccountDidNotDeserialize NEW"]
    C -- NO --> E{"disc bytes match?"}
    E -- NO --> F["Err: AccountDiscriminatorMismatch"]
    E -- YES --> G["bytemuck::from_bytes safe cast"]

    H["#account zero - generate_constraint_zeroed"] --> I["try_borrow_data"]
    I --> J{"data.len() less than disc.len()?"}
    J -- YES --> K["Err: AccountDidNotDeserialize NEW"]
    J -- NO --> L{"discriminator all-zero?"}
    L -- NO --> M["Err: ConstraintZero"]
    L -- YES --> N["from_account_info"]
    N --> O["try_from_unchecked - owner check only"]
    O --> P["User calls load_init later"]
    P --> C
Loading

Comments Outside Diff (2)

  1. lang/src/accounts/account_loader.rs, line 159-172 (link)

    P2 The consolidated size check changes the error code for accounts whose data is shorter than the discriminator itself. Previously load() / load_mut() returned AccountDiscriminatorNotFound for that sub-case; now they return AccountDidNotDeserialize for all undersized data. Any caller that matched specifically on AccountDiscriminatorNotFound from these methods (e.g. in error-handling tests or program logic) will silently receive a different variant. Consider preserving the finer-grained error by splitting the check, or at least documenting the change in the function doc comment.

  2. lang/src/accounts/account_loader.rs, line 187-200 (link)

    P2 Same as load(): the combined check drops the fine-grained AccountDiscriminatorNotFound error for accounts shorter than the discriminator, replacing it with AccountDidNotDeserialize for all undersized inputs. Splitting the check keeps the error semantics consistent with try_from() and with the existing public contract.

Reviews (1): Last reviewed commit: "Update account_loader_truncation.rs" | Re-trigger Greptile

Comment thread lang/syn/src/codegen/accounts/constraints.rs Outdated
@jamie-osec
Copy link
Copy Markdown
Collaborator

There's a lot of repeated checks here, can we factor this out?

@Abhilashpatel12
Copy link
Copy Markdown
Author

Good point , I’ll factor the repeated validation logic into shared private helper methods in account_loader.rs to reduce duplication while preserving the existing error behavior. I’ll also clean up the discriminator-length naming in the generated constraint path for clarity.

@Abhilashpatel12
Copy link
Copy Markdown
Author

@jamie-osec please checkout the updated commits

Comment thread lang/src/accounts/account_loader.rs
Co-authored-by: Jamie Hill-Daniel <134328753+jamie-osec@users.noreply.github.com>
@Abhilashpatel12
Copy link
Copy Markdown
Author

Abhilashpatel12 commented May 14, 2026

@jamie-osec tests are passed needed authorization to pass the vercel test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants