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

1 stars 0 forks source link

QA Report #104

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[L] Arithmetic operations without using SafeMath may over/underflow

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

for (uint256 i = 0; i < _currentValidators.length; i++) {
    // If v is set to 0, this signifies that it was not possible to get a signature from this validator and we skip evaluation
    // (In a valid signature, it is either 27 or 28)
    if (_v[i] != 0) {
        // Check that the current validator has signed off on the hash
        require(
            verifySig(_currentValidators[i], _theHash, _v[i], _r[i], _s[i]),
            "Validator signature does not match."
        );

        // Sum up cumulative power
        cumulativePower = cumulativePower + _currentPowers[i];

        // Break early to avoid wasting gas
        if (cumulativePower > _powerThreshold) {
            break;
        }
    }
}

In L244, even though it's unlikely to overflow in this particular case, we still recommend using SafeMath instead.

[N] Outdated compiler version

It's a best practice to use the latest compiler version.

The specified minimum compiler version is quite old. Older compilers might be susceptible to some bugs. We recommend changing the solidity version pragma to the latest version to enforce the use of an up to date compiler.

List of known compiler bugs and their severity can be found here: https://etherscan.io/solcbuginfo

[N] Unused function

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

    function manageWhitelist(
        address[] memory _users,
        bool _isWhitelisted
        ) public onlyWhitelisted {
         for (uint256 i = 0; i < _users.length; i++) {
            require(
                _users[i] != address(0),
                "User is the zero address"
            );
            whitelisted[_users[i]] = _isWhitelisted;
        }
        emit WhitelistedStatusModified(msg.sender, _users, _isWhitelisted);
    }

[N] SendToCosmosEvent._destination can be changed to string rather than bytes32 to support different chains in the future to indicate almost anything else we may

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

    event SendToCosmosEvent(
        address indexed _tokenContract,
        address indexed _sender,
        bytes32 indexed _destination,
        uint256 _amount,
        uint256 _eventNonce
    );
V-Staykov commented 2 years ago

[L] Arithmetic operations without using SafeMath may over/underflow - on the Cosmos side we have a limit of 10 billion total voting power, so overflowing is not an issue here.