Skip to content

lang: adjust realloc implementation to safeguard max increase and idempotency#1986

Merged
armaniferrante merged 14 commits into
otter-sec:masterfrom
callensm:fix/realloc
Jul 5, 2022
Merged

lang: adjust realloc implementation to safeguard max increase and idempotency#1986
armaniferrante merged 14 commits into
otter-sec:masterfrom
callensm:fix/realloc

Conversation

@callensm

@callensm callensm commented Jun 18, 2022

Copy link
Copy Markdown
Contributor
  • includes the original implementation of the realloc constraint group from lang: add realloc constraint group #1943
  • adds a check on the __delta_bytes to ensure it is <= MAX_PERMITTED_DATA_INCREASE
  • add test to ensure duplicate accounts does not imply duplicate reallocation
  • adds new AccountReallocExceedsLimit error code to rust and typescript for the data increase limit check

@callensm callensm changed the title lang: adjust realloc implementation to safeguard max increase and idempotency [WIP] lang: adjust realloc implementation to safeguard max increase and idempotency Jun 18, 2022
@callensm callensm changed the title [WIP] lang: adjust realloc implementation to safeguard max increase and idempotency lang: adjust realloc implementation to safeguard max increase and idempotency Jun 18, 2022
@callensm

Copy link
Copy Markdown
Contributor Author

added reallocs: &mut std::collections::BTreeSet<String> as a new argument to Accounts::try_accounts in order to track public keys that are assigned to reallocation constraint groups in order to throw a duplicate account realloc error if the public key of an account is associated with more than one reallocation per instruction call.

Comment thread lang/syn/src/codegen/accounts/constraints.rs Outdated
Comment thread lang/derive/accounts/src/lib.rs
Comment thread lang/syn/src/codegen/accounts/constraints.rs Outdated
Comment thread lang/syn/src/codegen/accounts/constraints.rs Outdated

@ethanwu10 ethanwu10 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that coverage of #[state] is lacking for zerocopy. Also, the second typo in the ix.has_receiver case for state interface impls is dead (and thus also untested), since #[interface] does not allow receivers in any of the trait's methods.

Also, I didn't see anything in rustdoc indicating that #[state] is deprecated...

Comment thread lang/syn/src/codegen/program/handlers.rs Outdated
Comment thread lang/syn/src/codegen/program/handlers.rs Outdated
@callensm

Copy link
Copy Markdown
Contributor Author

tests/swap is passing when i run it locally with the debug build of the cli, not sure why its failing in the pipeline.

Comment thread lang/src/lib.rs Outdated
accounts: &mut &[AccountInfo<'info>],
ix_data: &[u8],
bumps: &mut BTreeMap<String, u8>,
reallocs: &mut BTreeSet<String>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since AFAICT nothing in docs signals that the Accounts trait is internal, this is a breaking change (with no real way around it if we want to implement the duplicate checking). It'd probably be a better idea to pass a new &mut TryAccountsContext struct or similar so that we can add more bits of state or data without needing to further modify this signature. Consider:

#[derive(Default)]
pub struct TryAccountsContext {
    // private field to prevent the struct from containing only public fields; prevents users from
    // constructing using a literal or pattern-matching/destructuring against this type
    _marker: (),
    pub bumps: BTreeMap<String, u8>,
    pub reallocs: BTreeSet<String>,
}

Comment thread lang/src/lib.rs Outdated
Comment thread CHANGELOG.md Outdated
@vercel

vercel Bot commented Jul 4, 2022

Copy link
Copy Markdown

@armaniferrante is attempting to deploy a commit to the 200ms Team on Vercel.

A member of the Team first needs to authorize it.

@edmbn

edmbn commented Jul 4, 2022

Copy link
Copy Markdown

added reallocs: &mut std::collections::BTreeSet<String> as a new argument to Accounts::try_accounts in order to track public keys that are assigned to reallocation constraint groups in order to throw a duplicate account realloc error if the public key of an account is associated with more than one reallocation per instruction call.

Will this mean we won't be able to reallocate an account 2 times in the same instruction? We might want to reallocate an account with 0 space and then with try_accounts reallocate to original space to have the account "reinitialized".

@armaniferrante armaniferrante merged commit c47fb28 into otter-sec:master Jul 5, 2022
@callensm callensm deleted the fix/realloc branch July 5, 2022 19:54
Otter-0x4ka5h pushed a commit to Otter-0x4ka5h/anchor that referenced this pull request Mar 25, 2026
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.

5 participants