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

NEO contract violates security constraints (a bit) #3596

Open
roman-khimov opened this issue Nov 24, 2024 · 2 comments
Open

NEO contract violates security constraints (a bit) #3596

roman-khimov opened this issue Nov 24, 2024 · 2 comments
Assignees
Labels
Bug Used to tag confirmed bugs Hardfork
Milestone

Comments

@roman-khimov
Copy link
Contributor

roman-khimov commented Nov 24, 2024

Describe the bug
We've added some events to NEO contract in #2754, but RequiredCallFlags of the respective methods (registerCandidate/unregisterCandidate, vote) remained the same. So these methods require States only, while in fact they emit notifications now and thus technically require AllowNotify.

Practically it's not a big problem, not a lot of people complaining, but still it's a violation of security constraints provided by call flags.

To Reproduce
Call any of the methods above with States call flags, get an event for free.

Expected behavior
Failure in a case of such call.

Additional context
While it's rather easy to fix with three lines changed, I fear we'd get something similar to #2673 and this behavior change can be probed by transactions, so it needs a hardfork. But can we actually change call flags with a hardfork? We can add/delete methods, but it looks like this case is somewhat different and can not be easily expressed currently, so some change to the hardfork mechanism is required in fact.

@shargon
Copy link
Member

shargon commented Nov 24, 2024

But can we actually change call flags with a hardfork?

Currently not, I will try to find the way

@Jim8y Jim8y added Bug Used to tag confirmed bugs Hardfork labels Nov 24, 2024
@Jim8y Jim8y added this to the v3.8.0 milestone Nov 24, 2024
@AnnaShaleva
Copy link
Member

A nice catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Used to tag confirmed bugs Hardfork
Projects
None yet
Development

No branches or pull requests

4 participants