Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4-initialization lint notes #19

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

oslfmt
Copy link
Contributor

@oslfmt oslfmt commented Aug 16, 2022

This PR is not meant to be merged (but could be in the future if a lint strategy is created). The only notable file in this PR is in ui/recommended/init_expansion.rs. I was trying to figure out what exactly anchor does to fix this exploit with their [#account(init)] macro, and see if it might provide some insight into what exactly "initialization" of an account means. This file contains the macro expansion (all of it happens in the try_accounts() method as far as I can tell). The init attribute expands to a large block of code, and I've made comments on the parts I found more interesting.

Some other miscellaneous notes:

  • Initialization seems to have to do something with allocating space. This happens in both the Anchor expansion and in the SPL token library
  • In the Anchor expansion, there is different logic depending on if the account has lamports or not. If not, it just calls the system program to create an instruction for creating accounts. But if it does have some lamports, it does some other actions like allocating space and assigning an owner. This is interesting and I'm not sure why they do this.
  • This function here is in the SPL token library and processes an "initialize_account" instruction. With regards to initialization, (one of) the important lines is here, which, as the comment above mentions, checks if the account is already initialized or not. The check in reference happens exactly on this line, which calls is_initialized. That checks if the account.state is equal to a certain enum variant. Thus, the Account struct uses an enum to tell if an account has already been initialized. However, a bool could just as easily be used, as is done in the Mint struct here. So a common "initialization" flag may be either an enum or bool. I can't think of an easy way to detect if the enum/bool is meant to be used as as initialization flag or not though, besides checking it's Ident??

In summary, a lint for this exploit could be made by:

  1. Finding out what exactly initialization of an account entails, and use this to detect an initialization happening.
  2. Check if the AccountInfo.data struct contains an "initialization flag". The SPL token library has a flag in their Mint and Account structs, and it seems like this is a reasonable way to not reinitialize accounts.

@oslfmt oslfmt marked this pull request as draft August 16, 2022 02:15
@oslfmt oslfmt marked this pull request as ready for review August 16, 2022 02:17
@smoelius smoelius marked this pull request as draft August 22, 2022 10:51
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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