Cudos contest - sorrynotsorry's results

Lines of code

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L453-L456 https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L568-L573 https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L579-L581

Vulnerability details

Impact

Calls inside loops that may address DoS.

Proof of Concept

Calls to external contracts inside a loop are dangerous (especially if the loop index can be user-controlled) because it could lead to DoS if one of the calls reverts or execution runs out of gas. Reference

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

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

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

Tools Used

Manual Review

Recommended Mitigation Steps

Avoid combining multiple calls in a single transaction, especially when calls are executed as part of a loop Always assume that external calls can fail Implement the contract logic to handle failed calls

QA (LOW - NON-CRITICAL)

• Floating Pragma used in CosmosToken.sol, Gravity.sol. Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma (i.e. by not using ^) helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively. Reference

• CosmosToken.sol, Gravity.sol files are having Solidity compiler version is 0.6.6 which is outdated. Currently the compiler version is v0.8.13.

• It's recommended to deploy the contracts with below Solidity compilers;

  0.5.16 - 0.5.17 0.6.11 - 0.6.12 0.7.5 - 0.7.6 0.8.4 - 0.8.7 Use a simple pragma version that allows any of these versions. Consider using the latest version of Solidity for testing. Reference

• At Gravity.sol , an expensive loop is used by incrementing state_variable in a loop incurs a lot of gas because of expensive SSTOREs, which might lead to an out-of-gas.

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);
	}

• The self notes on Gravity.sol#L137-#138 to be removed.

• EVM's ecrecover is susceptible to signature malleability which allows replay attacks, but using OpenZeppelin’s ECDSA library can be mitigation in Gravity.sol for verifySig() function. Reference

• At Gravity.sol, submitBatch() function, _destinations param should be checked for having address(0) inside.

GAS OPTIMIZATIONS

• At CosmosToken.sol, the state variable MAX_UINT can be declared as constant / immutable to save gas.

• Using revert strings greater than 32 bytes will increase the gas consumption, custom errors (at v0.8.0) or enumerable sets can be used to decrease the consumption.

• At Gravity.sol , an expensive loop is used by incrementing state_variable in a loop incurs a lot of gas because of expensive SSTOREs, which might lead to an out-of-gas.

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);
	}