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

Solidity valset update needs sanity check #346

Open
andrey-kuprianov opened this issue Oct 5, 2021 · 0 comments
Open

Solidity valset update needs sanity check #346

andrey-kuprianov opened this issue Oct 5, 2021 · 0 comments

Comments

@andrey-kuprianov
Copy link

Surfaced from @informalsystems audit of Althea Gravity Bridge at commit 19a4cfe

severity: Low
type: Implementation bug
difficulty: Intermediate

Involved artifacts

Description

The Solidity contract function updateValset() checks that the new validator set is signed by enough validators with voting power from the current validator set. After that it updates the stored valset to the new valset unconditionally. Some sanity checks are missing, and needs to be implemented:

  • the new validator set should not be empty: if an empty validator set is accepted, anyone can take control of the Gravity Bridge;
  • the new validator set should have enough voting power: similar to the constructor check. This check subsumes the above non-emptiness check.

Problem Scenarios

Due to an unforeseen condition, or a bug in the Cosmos SDK module or Orchestrator the following could happen:

  • an empty valset could be submitted; in that case anyone could take control of the bridge;
  • a valset with not enough voting power could be submitted; in that case any further attempts to update it will fail, and the bridge will be locked.

Recommendation

Add the aforementioned sanity check to the Solidity contract, rejecting the validator set update if the sanity check fails.

@andrey-kuprianov andrey-kuprianov changed the title Solidity valet update needs sanity check Solidity valset update needs sanity check Oct 5, 2021
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

No branches or pull requests

1 participant