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 #257

Open
EricBolten opened this issue Oct 21, 2021 · 1 comment
Open

Solidity valset update needs sanity check #257

EricBolten opened this issue Oct 21, 2021 · 1 comment

Comments

@EricBolten
Copy link
Contributor

Original issue

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.

@hannydevelop
Copy link
Contributor

This issue is for the Solidity team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants