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

[contract-deploy] Ensure there are no private functions with duplicate selectors #4433

Open
Tracked by #4052
spalladino opened this issue Feb 5, 2024 · 3 comments
Open
Tracked by #4052

Comments

@spalladino
Copy link
Collaborator

spalladino commented Feb 5, 2024

We want to ensure there are no two private functions in the same contract with the same selector. However, it's unclear where we can verify this. A malicious user could register a class with a function tree that contains duplicates, and we have no way to catch it. We could reject this at the pxe, so that when it receives a contract artifact, it checks for duplicates. Note that, if we allow a dapp to register partial artifacts (ie not all private functions) it may still be possible for an attacker to squeeze in duplicate selectors. In Mike's words:

check during class registration that there are no diplicate function selectors for private functions. Otherwise the acir for one function selector could be confused with the acir for another function selector, and that could be bad.

@github-project-automation github-project-automation bot moved this to Todo in A3 Feb 5, 2024
@spalladino spalladino changed the title Ensure there are no private functions with duplicate selectors [contract-deploy] Ensure there are no private functions with duplicate selectors Aug 26, 2024
@nventuro
Copy link
Contributor

nventuro commented Nov 4, 2024

Note that, if we allow a dapp to register partial artifacts (ie not all private functions) it may still be possible for an attacker to squeeze in duplicate selectors

What is the usecase for partial registration? How could you trust something you've only seen part of?

@spalladino
Copy link
Collaborator Author

True, I don't see any real use case for that.

@sklppy88
Copy link
Contributor

After discussion with @nventuro and @spalladino, it seems like we will want this check in three places.

  1. We will want to the noir compilation for contracts to fail if duplicate selectors are detected
  2. The contract class registerer should fail when trying to register a contract with duplicate selectors
  3. PXE should throw an error when trying to register a contract with duplicate selectors

With this in mind, I have made three sub-issues for this issue.

sklppy88 added a commit that referenced this issue Nov 20, 2024
… private function selectors (#9773)

Related to #4433

This PR adds pretty naive validation when adding a contract artifact to
the pxe database. Specifically it rejects adding contract artifacts to
the PXE with duplicate private function selectors.

This also assumes the absence of partial artifacts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

3 participants