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

Account ID can be reused #649

Closed
arhag opened this issue Sep 5, 2023 · 0 comments · Fixed by #648 or #652
Closed

Account ID can be reused #649

arhag opened this issue Sep 5, 2023 · 0 comments · Fixed by #648 or #652
Assignees
Labels
bug Something isn't working 👍 lgtm
Milestone

Comments

@arhag
Copy link
Member

arhag commented Sep 5, 2023

Using available_primary_key from multi_index to determine the account ID of a new account is not safe since an account can be deleted (via selfdestruct) which can cause the new account to reuse an account ID that was previously used by another account.

This can cause a serious issue because contract state is not immediately erased but is instead marked for garbage collection. The expectation with the garbage collection mechanism was that account IDs would never be repeated. But if they are repeated, it is possible for a new contract account to inherit the state from a deleted contract account (the state as of when the contract called selfdestruct or possibly some subset of it if the gc action was called).

A simple solution to avoid repeating the account ID is to keep track of the next account ID to use in a variable next_account_id stored in a singleton table. (This could be in a new table for that purpose or it could be added as a binary extension to the existing config table.) Instead of using available_primary_key(), the contract would use next_account_id and would ensure it is incremented so that subsequent created accounts continue using an appropriate unique ID.

This bug should be fixed in a way that is compatible with the contracts already deployed on testnet and mainnet, whether this bug was exploited or not on those networks. This means it should be possible for the contract to determine whether a next_account_id exists already or not; it would not exist if this is the first time a new account is created after deployment of a contract that includes the code changes to fix this bug. If it does not exist, it is important to set an initial value that is likely to be safe. For example the value of available_primary_key() could be used as the initial value.

@arhag arhag added the bug Something isn't working label Sep 5, 2023
@github-project-automation github-project-automation bot moved this to Todo in EOS EVM Sep 5, 2023
@arhag arhag added this to the 0.5.2 milestone Sep 5, 2023
@arhag arhag added the 👍 lgtm label Sep 5, 2023
@arhag arhag linked a pull request Sep 5, 2023 that will close this issue
@elmato elmato linked a pull request Sep 5, 2023 that will close this issue
@github-project-automation github-project-automation bot moved this from Todo to Done in EOS EVM Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 👍 lgtm
Projects
Status: Done
2 participants