Axelar Network v2 contest - CodingNameKiki's results

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L23 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L51 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L70 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L86 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L128 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L144 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L63

Vulnerability details

Impact

The use of the deprecated transfer() function for an address will inevitably make the transaction fail if: 1.The claimer smart contract does not implement a payable function. 2.The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit. 3.The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call’s gas usage above 2300. Note - using higher than 2300 gas might be mandatory for some multisig wallets.

Proof of Concept

Check the links above

Recommended Mitigation Steps

Replace transfer() with call(). Keep in mind to check whether the call was successful by validating the return value.

1. x += y cost more gas than x= x + y for state variables

AxelarAuthWeighted.sol 70: totalWeight += newWeights[i]; 105: weight += weights[operatorIndex];

2. abi.encode() is less efficient than abi.encodePacked()

AxelarAuthWeighted.sol 32: bytes32 operatorsHash = keccak256(abi.encode(operators, weights, threshold));

3. Internal functions only called once can be inlined to save gas

AxelarAuthWeighted.sol 115: function _isSortedAscAndContainsNoDuplicate(address[] memory accounts) internal pure returns (bool) {

4. ++i cost less gas than i++, especially when it's used in for loops (--i,i--) aswell.

AxelarDepositService.sol 114: for (uint256 i; i < refundTokens.length; i++) { 168: for (uint256 i; i < refundTokens.length; i++) { 204: for (uint256 i; i < refundTokens.length; i++) {

5. Internal functions only called once can be inlined to save gas

AxelarGateway.sol 611: function _setTokenDailyMintAmount(string memory symbol, uint256 amount) internal { 622: function _setTokenAddress(string memory symbol, address tokenAddress) internal { 655: function _setImplementation(address newImplementation) internal {

6. Empty block should be removed or emit something

AxelarAuthWeighted.sol 101: for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}

7. Empty blocks should be removed or emit something

ReceiverImplementation.sol 12: constructor(address gateway, string memory wrappedSymbol) DepositBase(gateway, wrappedSymbol) {}

8. Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

AxelarAuthWeighted.sol 14: uint8 internal constant OLD_KEY_RETENTION = 16;