Cudos contest - gzeon's results

Low Risk

Unsafe cumulativePower math

cumulativePower can overflow such that total powers is more than cumulativePower

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L661-L662

			cumulativePower = cumulativePower + _powers[i];

verifySig will return true for invalid signature from address(0)

ecrecover return 0 for invalid signature

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L175-L186

	function verifySig(
		address _signer,
		bytes32 _theHash,
		uint8 _v,
		bytes32 _r,
		bytes32 _s
	) private pure returns (bool) {
		bytes32 messageDigest = keccak256(
			abi.encodePacked("\x19Ethereum Signed Message:\n32", _theHash)
		);
		return _signer == ecrecover(messageDigest, _v, _r, _s);
	}

Non-Critical

Pin Solidity Version

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L1-L2

pragma solidity ^0.6.6;

Inconsistant Solidity Version

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L1-L2

pragma solidity ^0.6.6;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/CudosAccessControls.sol#L3-L4

pragma solidity 0.6.12;

Variable can be constant

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/CosmosToken.sol#L5-L6

	uint256 MAX_UINT = 2**256 - 1;

Variables can be immutable

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L59-L63

	// These are set once at initialization
	bytes32 public state_gravityId;
	uint256 public state_powerThreshold;

	CudosAccessControls public cudosAccessControls;

Cache state_lastEventNonce

Here is an example but there are multiple occurrence https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L583-L592

Change the code into

		// LOGS scoped to reduce stack depth
		{
            uint 256 _state_lastEventNonce = state_lastEventNonce.add(1);
			state_lastEventNonce = _state_lastEventNonce;
			emit LogicCallEvent(
				_args.invalidationId,
				_args.invalidationNonce,
				returnData,
				_state_lastEventNonce
			);
		}

For loop optimization with ++i

using ++i save gas against i++

contracts/Gravity.sol:128:               for (uint256 i = 0; i < _users.length; i++) {
contracts/Gravity.sol:233:              for (uint256 i = 0; i < _currentValidators.length; i++) {
contracts/Gravity.sol:263:              for (uint256 i = 0; i < _newValset.validators.length; i++) {
contracts/Gravity.sol:453:                              for (uint256 i = 0; i < _amounts.length; i++) {
contracts/Gravity.sol:568:              for (uint256 i = 0; i < _args.transferAmounts.length; i++) {
contracts/Gravity.sol:579:              for (uint256 i = 0; i < _args.feeAmounts.length; i++) {
contracts/Gravity.sol:660:              for (uint256 i = 0; i < _powers.length; i++) {

For loop optimization by caching length

Cache array length to reduce MLOADs, e.g.

uint256 arrlen = _users.length;
for (uint256 i = 0; i < arrlen; i++)

contracts/Gravity.sol:128:               for (uint256 i = 0; i < _users.length; i++) {
contracts/Gravity.sol:233:              for (uint256 i = 0; i < _currentValidators.length; i++) {
contracts/Gravity.sol:263:              for (uint256 i = 0; i < _newValset.validators.length; i++) {
contracts/Gravity.sol:453:                              for (uint256 i = 0; i < _amounts.length; i++) {
contracts/Gravity.sol:568:              for (uint256 i = 0; i < _args.transferAmounts.length; i++) {
contracts/Gravity.sol:579:              for (uint256 i = 0; i < _args.feeAmounts.length; i++) {
contracts/Gravity.sol:660:              for (uint256 i = 0; i < _powers.length; i++) {

Unnecessary safe math

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L465-L466

contracts/Gravity.sol:349:              state_lastEventNonce = state_lastEventNonce.add(1);
contracts/Gravity.sol:465:                      state_lastEventNonce = state_lastEventNonce.add(1);
contracts/Gravity.sol:585:                      state_lastEventNonce = state_lastEventNonce.add(1);
contracts/Gravity.sol:601:              state_lastEventNonce = state_lastEventNonce.add(1);
contracts/Gravity.sol:621:              state_lastEventNonce = state_lastEventNonce.add(1);

Use separate require instead of &&

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L301-L307

		require(
			_currentValset.validators.length == _currentValset.powers.length &&
				_currentValset.validators.length == _v.length &&
				_currentValset.validators.length == _r.length &&
				_currentValset.validators.length == _s.length,
			"Malformed current validator set"
		);

Upgrade to latest Solidity

Consider upgrade to latest 0.8.12 for various gas saving