Axelar Network v2 contest - sashik_eth's results

Lines of code

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

Vulnerability details

Impact

Multiple times in contracts used transfer() function for sending ether instead of recommended call(). It could lead to broken functionality in the future: https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Recommended Mitigation Steps

Use call() function instead of transfer() with checking successful return result and reentrancy guard.

G01 - unchecked block can be used for gas efficiency of the expression that can't overflow/underflow

contracts/AxelarGateway.sol:157 return getUint(_getTokenDailyMintAmountKey(symbol, block.timestamp / 1 days)); 
contracts/AxelarGateway.sol:615 _setUint(_getTokenDailyMintAmountKey(symbol, block.timestamp / 1 days), amount); 

G02 - custom errors declared but never used

Using custom errors istead of revert strings could save gas: https://blog.soliditylang.org/2021/04/21/custom-errors/

xc20/contracts/XC20Wrapper.sol:12   error TransferFailed(); 
xc20/contracts/XC20Wrapper.sol:13    error NotAxelarToken();
xc20/contracts/XC20Wrapper.sol:14    error NotXc20Token();
xc20/contracts/XC20Wrapper.sol:15    error InsufficientBalance();
xc20/contracts/XC20Wrapper.sol:16    error AlreadyWrappingAxelarToken();
xc20/contracts/XC20Wrapper.sol:17    error AlreadyWrappingXC20Token();
xc20/contracts/XC20Wrapper.sol:18    error NotOwnerOfXc20();

G03 - Optimised for loops

Loop could save gas if:

1. Cache condition value
2. Use prefix ++i increment
3. Put increment inside unchecked block It would have next view:

	uint256 length = myStateVarOrArrayLength;
	for (uint256 i = 0; i < length;) {
		// ...
		unchecked {
			++i;
		}
	}

Next loops could be optimized:

contracts/AxelarGateway.sol:195 for (uint256 i; i < adminCount; ++i) { 
contracts/AxelarGateway.sol:207 for (uint256 i = 0; i < symbols.length; i++) { 
contracts/AxelarGateway.sol:292 for (uint256 i; i < commandsLength; ++i) { 
contracts/auth/AxelarAuthWeighted.sol:17    for (uint256 i; i < recentOperators.length; ++i) { 
contracts/auth/AxelarAuthWeighted.sol:69    for (uint256 i = 0; i < weightsLength; ++i) { 
contracts/auth/AxelarAuthWeighted.sol:98    for (uint256 i = 0; i < signatures.length; ++i) { 
contracts/auth/AxelarAuthWeighted.sol:101   for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {} 
contracts/auth/AxelarAuthWeighted.sol:116   for (uint256 i; i < accounts.length - 1; ++i) { 
contracts/deposit-service/AxelarDepositService.sol:114  for (uint256 i; i < refundTokens.length; i++) { 
contracts/deposit-service/AxelarDepositService.sol:168  for (uint256 i; i < refundTokens.length; i++) { 
contracts/deposit-service/AxelarDepositService.sol:204  for (uint256 i; i < refundTokens.length; i++) { 
contracts/gas-service/AxelarGasService.sol:123  for (uint256 i; i < tokens.length; i++) {