code-423n4 / 2022-05-cudos-findings

1 stars 0 forks source link

No cumulative power check when updating valset #77

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cudos/blob/main/solidity/contracts/Gravity.sol#L276

Vulnerability details

Impact

When batches are submitted by validators in the current valset, they are checked for validity based on signatures and cumulative powers. Each validator in the valset has an associated power which can give certain validators more voting power than others. For a batch to be valid, the cumulative power of all "YES" signatures needs to be greater than _powerThreshold, which is set once during contract initialisation in the constructor.

In the constructor, there is a check for cumulative power here. However, when updating a valset, there is no check for the cumulative power of the new valset. Therefore it is theoretically possible to submit a valset that doesn't have enough cumulative power to succeed in any transactions, and then the bridge will be locked (since the valset can't be updated either).

Proof of Concept

In updateValset there is no logic for checking the cumulative power of the new valset. See below for recommended actions to resolve this.

Tools Used

VSCode

Recommended Mitigation Steps

A check for cumulative power of the new valset should be added to updateValset:

        uint256 cumulativePower = 0;
        for (uint256 i = 0; i < _newValset.powers.length; i++) {
            cumulativePower = cumulativePower + _newValset.powers[i];
            if (cumulativePower > _powerThreshold) {
                break;
            }
        }
        require(
            cumulativePower > _powerThreshold,
            "Submitted validator set signatures do not have enough power."
        );
V-Staykov commented 2 years ago

Duplicate of #123