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

Assert isNew in the deploy method of a token contract #1439

Closed
mitschabaude opened this issue Feb 16, 2024 · 0 comments · Fixed by #1446
Closed

Assert isNew in the deploy method of a token contract #1439

mitschabaude opened this issue Feb 16, 2024 · 0 comments · Fixed by #1446
Assignees

Comments

@mitschabaude
Copy link
Member

mitschabaude commented Feb 16, 2024

The problem

Token contracts are able to ensure validity of token interactions by the requirement that account updates of them always have to be included as a parent, for every token interaction.

This only works because we have the 'access' permission!

For default accounts, everyone can include one of their account updates without authorization (common use case: sending someone money without their authorization). The 'access' permission in that case is "none".

Token contracts set their access permission to "proof" or "proofOrSignature". That way, an account update of them always requires a proof. Which ensures that you have to successfully execute a token contract method to approve any token interaction. Usually, the access permission is set to "proof" in the deploy() method.

There is a potential security hole. Imagine someone creates an account, and some time later deploys a token contract to it. In the meantime, the access permission is still "none". An attacker who anticipates that this will become a token contract could mint tokens in the meantime. For example, such an attacker could monitor the mempool for any transactions that look like token contract deployments, and try to frontrun them with a high-fee transaction minting themselves tokens.

The mitigation

A solid defense against this attack is to require the isNew precondition on the same account update that sets the access permission. That means the token contract must be secured from the very start of its account's existence. (The worst case is that the token contract can't be deployed, because an attacker created the account earlier.)

This is a subtle issue, so we should mitigate it by default, otherwise many token contracts would be vulnerable to this attack in practice.

The measure I recommend is to this.account.isNew.requireEquals(Bool(true)) inside the default deploy() on TokenContract.

@mitschabaude mitschabaude added breaking Issues that will lead to breaking changes and removed breaking Issues that will lead to breaking changes labels Feb 16, 2024
@mitschabaude mitschabaude self-assigned this Feb 19, 2024
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 a pull request may close this issue.

1 participant