Cudos contest - danb's results

Lines of code

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

Vulnerability details

In the constructor, it is checked that the cumulative power is greater than the threshold:

uint256 cumulativePower = 0;
for (uint256 i = 0; i < _powers.length; i++) {
	cumulativePower = cumulativePower + _powers[i];
	if (cumulativePower > _powerThreshold) {
		break;
	}
}
require(
	cumulativePower > _powerThreshold,
	"Submitted validator set signatures do not have enough power."
);

However, updateValset doesn't have this check. therefore it's possible to change the contract state such that the submitted validator set signatures do not have enough power

Recommended Mitigation Steps

add this check to updateValset

Lines of code

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

Vulnerability details

the admin can steal all the money.

Lines of code

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

Vulnerability details

In the constructor, cumulativePower can overflow. If the powers sum overflows, it will make the contract malfunction, because the cumulativePower in checkValidatorSignatures may overflow, and then it's possible that more votes in favor, will make the proposal fail.

Proof of Concept

let's assume the powers sum overflow. the are two validators. In a proposal, in checkValidatorSignatures, the first validator's power is MAX_UINT, therefore the proposal should pass, but the second validator power is 1, and when it's added to it, it becomes 0, and the proposal fails.

Recommended Mitigation Steps

use safe math of openzeppelin at line 661