Axelar Network v2 contest - IllIllI's results

Decentralized interoperability network.

General Information

Platform: Code4rena

Start Date: 29/07/2022

Pot Size: $50,000 USDC

Total HM: 6

Participants: 75

Period: 5 days

Judge: GalloDaSballo

Total Solo HM: 3

Id: 149

League: ETH

Axelar Network

Findings Distribution

Researcher Performance

Rank: 10/75

Findings: 2

Award: $529.69

🌟 Selected for report: 1

šŸš€ Solo Findings: 0

Summary

Low Risk Issues

IssueInstances
[L‑01]Don't use payable.transfer()/payable.send()4
[L‑02]Unused/empty receive()/fallback() function2
[L‑03]Missing checks for address(0x0) when assigning values to address state variables2
[L‑04]abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()5

Total: 13 instances over 4 issues

Non-critical Issues

IssueInstances
[N‑01]Return values of approve() not checked3
[N‑02]public functions not called by the contract should be declared external instead1
[N‑03]constants should be defined rather than using magic numbers2
[N‑04]Use a more recent version of solidity2
[N‑05]Expressions for constant values such as a call to keccak256(), should use immutable rather than constant13
[N‑06]File is missing NatSpec10
[N‑07]Event is missing indexed fields7

Total: 38 instances over 7 issues

Low Risk Issues

[L‑01] Don't use payable.transfer()/payable.send()

The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient is either an EOA account, or is a contract that has a payable callback. For the contract case, the transfer() call only provides 2300 gas for the contract to complete its operations. This means the following cases can cause the transfer to fail:

  • The contract does not have a payable callback
  • The contract's payable callback spends more than 2300 gas (which is only enough to emit something)
  • The contract is called through a proxy which itself uses up the 2300 gas Use OpenZeppelin's Address.sendValue() instead

There are 4 instances of this issue:

File: contracts/deposit-service/ReceiverImplementation.sol

23:           if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

51:               if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

71:           if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

86:           recipient.transfer(amount);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L23

[L‑02] Unused/empty receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))

There are 2 instances of this issue:

File: contracts/deposit-service/AxelarDepositServiceProxy.sol

13:       receive() external payable override {}

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositServiceProxy.sol#L13

File: contracts/deposit-service/DepositReceiver.sol

29:       receive() external payable {}

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol#L29

[L‑03] Missing checks for address(0x0) when assigning values to address state variables

There are 2 instances of this issue:

File: contracts/AxelarGateway.sol

52:           AUTH_MODULE = authModule;

53:           TOKEN_DEPLOYER_IMPLEMENTATION = tokenDeployerImplementation;

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L52

[L‑04] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.

If all arguments are strings and or bytes, bytes.concat() should be used instead

There are 5 instances of this issue:

File: contracts/AxelarGateway.sol

342:              bytes32 salt = keccak256(abi.encodePacked(symbol));

540:          return keccak256(abi.encodePacked(PREFIX_TOKEN_DAILY_MINT_LIMIT, symbol));

544:          return keccak256(abi.encodePacked(PREFIX_TOKEN_DAILY_MINT_AMOUNT, symbol, day));

548:          return keccak256(abi.encodePacked(PREFIX_TOKEN_TYPE, symbol));

552:          return keccak256(abi.encodePacked(PREFIX_TOKEN_ADDRESS, symbol));

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L342

Non-critical Issues

[N‑01] Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

There are 3 instances of this issue:

File: contracts/deposit-service/AxelarDepositService.sol

30:           IERC20(wrappedTokenAddress).approve(gateway, amount);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L30

File: contracts/deposit-service/ReceiverImplementation.sol

38:           IERC20(tokenAddress).approve(gateway, amount);

64:           IERC20(wrappedTokenAddress).approve(gateway, amount);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L38

[N‑02] public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

There is 1 instance of this issue:

File: contracts/deposit-service/AxelarDepositService.sol

241:      function contractId() public pure returns (bytes32) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L241

[N‑03] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 2 instances of this issue:

File: contracts/deposit-service/AxelarDepositService.sol

/// @audit 0xff
229:                                  bytes1(0xff),

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L229

File: contracts/deposit-service/DepositReceiver.sol

/// @audit 0x40
18:                   let ptr := mload(0x40)

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol#L18

[N‑04] Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

There are 2 instances of this issue:

File: contracts/AxelarGateway.sol

3:    pragma solidity 0.8.9;

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L3

File: contracts/deposit-service/AxelarDepositService.sol

3:    pragma solidity 0.8.9;

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L3

[N‑05] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

While it doesn't save any gas because the compiler knows that developers often make this mistake, it's still best to use the right tool for the task at hand. There is a difference between constant variables and immutable variables, and they should each be used in their appropriate contexts. constants should be used for literal values written into the code, and immutable variables should be used for expressions, or values calculated in, or passed into the constructor.

There are 13 instances of this issue:

File: contracts/AxelarGateway.sol

30:       bytes32 internal constant PREFIX_COMMAND_EXECUTED = keccak256('command-executed');

31:       bytes32 internal constant PREFIX_TOKEN_ADDRESS = keccak256('token-address');

32:       bytes32 internal constant PREFIX_TOKEN_TYPE = keccak256('token-type');

33:       bytes32 internal constant PREFIX_CONTRACT_CALL_APPROVED = keccak256('contract-call-approved');

34:       bytes32 internal constant PREFIX_CONTRACT_CALL_APPROVED_WITH_MINT = keccak256('contract-call-approved-with-mint');

35:       bytes32 internal constant PREFIX_TOKEN_DAILY_MINT_LIMIT = keccak256('token-daily-mint-limit');

36:       bytes32 internal constant PREFIX_TOKEN_DAILY_MINT_AMOUNT = keccak256('token-daily-mint-amount');

38:       bytes32 internal constant SELECTOR_BURN_TOKEN = keccak256('burnToken');

39:       bytes32 internal constant SELECTOR_DEPLOY_TOKEN = keccak256('deployToken');

40:       bytes32 internal constant SELECTOR_MINT_TOKEN = keccak256('mintToken');

41:       bytes32 internal constant SELECTOR_APPROVE_CONTRACT_CALL = keccak256('approveContractCall');

42:       bytes32 internal constant SELECTOR_APPROVE_CONTRACT_CALL_WITH_MINT = keccak256('approveContractCallWithMint');

43:       bytes32 internal constant SELECTOR_TRANSFER_OPERATORSHIP = keccak256('transferOperatorship');

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L30

[N‑06] File is missing NatSpec

There are 10 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol

File: contracts/deposit-service/AxelarDepositServiceProxy.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositServiceProxy.sol

File: contracts/deposit-service/DepositReceiver.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol

File: contracts/gas-service/AxelarGasServiceProxy.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasServiceProxy.sol

File: contracts/gas-service/AxelarGasService.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol

File: contracts/interfaces/IAxelarAuth.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuth.sol

File: contracts/interfaces/IAxelarAuthWeighted.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuthWeighted.sol

File: contracts/interfaces/IAxelarDepositService.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarDepositService.sol

File: contracts/interfaces/IAxelarExecutable.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarExecutable.sol

File: contracts/interfaces/IAxelarGasService.sol

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol

[N‑07] Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (threefields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

There are 7 instances of this issue:

File: contracts/interfaces/IAxelarAuthWeighted.sol

14:       event OperatorshipTransferred(address[] newOperators, uint256[] newWeights, uint256 newThreshold);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuthWeighted.sol#L14

File: contracts/interfaces/IAxelarGasService.sol

13        event GasPaidForContractCall(
14            address indexed sourceAddress,
15            string destinationChain,
16            string destinationAddress,
17            bytes32 indexed payloadHash,
18            address gasToken,
19            uint256 gasFeeAmount,
20            address refundAddress
21:       );

23        event GasPaidForContractCallWithToken(
24            address indexed sourceAddress,
25            string destinationChain,
26            string destinationAddress,
27            bytes32 indexed payloadHash,
28            string symbol,
29            uint256 amount,
30            address gasToken,
31            uint256 gasFeeAmount,
32            address refundAddress
33:       );

35        event NativeGasPaidForContractCall(
36            address indexed sourceAddress,
37            string destinationChain,
38            string destinationAddress,
39            bytes32 indexed payloadHash,
40            uint256 gasFeeAmount,
41            address refundAddress
42:       );

44        event NativeGasPaidForContractCallWithToken(
45            address indexed sourceAddress,
46            string destinationChain,
47            string destinationAddress,
48            bytes32 indexed payloadHash,
49            string symbol,
50            uint256 amount,
51            uint256 gasFeeAmount,
52            address refundAddress
53:       );

55:       event GasAdded(bytes32 indexed txHash, uint256 indexed logIndex, address gasToken, uint256 gasFeeAmount, address refundAddress);

57:       event NativeGasAdded(bytes32 indexed txHash, uint256 indexed logIndex, uint256 gasFeeAmount, address refundAddress);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L13-L21

#0 - re1ro

2022-08-05T06:17:28Z

L1

Dup #4

L2 L3

Dup #3 (5)

L4

Dup #8 (5)

N1

Dup #3

N2

Ack

N3

Ack

N4 - N5

Dup #3

N5

Dup #12

N6

Ack

N7

Dup #3

#1 - GalloDaSballo

2022-08-31T23:46:49Z

[L‑01] Don't use payable.transfer()/payable.send()

L

[L‑02] Unused/empty receive()/fallback() function

Valid for DepositServiceProxy L

[L‑03] Missing checks for address(0x0) when assigning values to address state variables

L

[L‑04] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Disputed for the cases prevented, the only demo I've ever found looked like the following privilegedAccounts = [0x1, 0x2] plebAccounts = [0x3] newPriveleged = [0x1] newPlebs = [0x2, 0x3] abi.encodePacked(privilegedAccounts, plebAccounts) == abi.encodePacked(newPriveleged, newPlebs)

[N‑01] Return values of approve() not checked

Raising to L because of the revert with certain tokens

[N‑02] public functions not called by the contract should be declared external instead

R

##Ā [N‑03] constants should be defined rather than using magic numbers I'm going to dispute the 2 specific instances, while the first one can be argued, the second is the free memory pointer

[N‑04] Use a more recent version of solidity

R

[N‑05] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

Not true for a loooooooooooooong time -> https://twitter.com/GalloDaSballo/status/1543729080926871557

4L 2R

Summary

Gas Optimizations

IssueInstances
[G‑01]Using calldata instead of memory for read-only arguments in external functions saves gas7
[G‑02]Avoid contract existence checks by using solidity version 0.8.10 or later25
[G‑03]internal functions only called once can be inlined to save gas7
[G‑04]<array>.length should not be looked up in every loop of a for-loop7
[G‑05]++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops12
[G‑06]keccak256() should only need to be called on a specific string literal once4
[G‑07]Optimize names to save gas10
[G‑08]++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)5
[G‑09]Empty blocks should be removed or emit something2
[G‑10]Functions guaranteed to revert when called by normal users can be marked payable11

Total: 90 instances over 10 issues

Gas Optimizations

[G‑01] Using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved

There are 7 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

/// @audit recentOperators
16:       constructor(bytes[] memory recentOperators) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L16

File: contracts/AxelarGateway.sol

172:      function tokenFrozen(string memory) external pure override returns (bool) {

/// @audit executeData
447       function _unpackLegacyCommands(bytes memory executeData)
448           external
449           pure
450           returns (
451               uint256 chainId,
452               bytes32[] memory commandIds,
453               string[] memory commands,
454:              bytes[] memory params

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L172

File: contracts/deposit-service/AxelarDepositService.sol

/// @audit wrappedSymbol
18:       constructor(address gateway, string memory wrappedSymbol) DepositBase(gateway, wrappedSymbol) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L18

File: contracts/deposit-service/DepositReceiver.sol

/// @audit delegateData
8:        constructor(bytes memory delegateData) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol#L8

File: contracts/deposit-service/ReceiverImplementation.sol

/// @audit wrappedSymbol
12:       constructor(address gateway, string memory wrappedSymbol) DepositBase(gateway, wrappedSymbol) {}

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L12

File: contracts/gas-service/AxelarGasService.sol

/// @audit symbol
35        function payGasForContractCallWithToken(
36            address sender,
37            string calldata destinationChain,
38            string calldata destinationAddress,
39            bytes calldata payload,
40            string memory symbol,
41            uint256 amount,
42            address gasToken,
43            uint256 gasFeeAmount,
44:           address refundAddress

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L35-L44

[G‑02] Avoid contract existence checks by using solidity version 0.8.10 or later

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value

There are 25 instances of this issue:

File: contracts/AxelarGateway.sol

/// @audit validateProof()
268:          bool currentOperators = IAxelarAuth(AUTH_MODULE).validateProof(messageHash, proof);

/// @audit _unpackLegacyCommands()
275:          try AxelarGateway(this)._unpackLegacyCommands(data) returns (

/// @audit call()
320:              (bool success, ) = address(this).call(abi.encodeWithSelector(commandSelector, params[i], commandId));

/// @audit balanceOf()
385:                  abi.encodeWithSelector(IERC20.transfer.selector, address(this), IERC20(tokenAddress).balanceOf(address(depositHandler)))

/// @audit burn()
393:              IBurnableMintableCappedERC20(tokenAddress).burn(salt);

/// @audit mint()
481:              IBurnableMintableCappedERC20(tokenAddress).mint(account, amount);

/// @audit depositAddress()
525:                  IBurnableMintableCappedERC20(tokenAddress).depositAddress(bytes32(0)),

/// @audit burn()
532:          IBurnableMintableCappedERC20(tokenAddress).burn(bytes32(0));

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L268

File: contracts/deposit-service/AxelarDepositService.sol

/// @audit approve()
30:           IERC20(wrappedTokenAddress).approve(gateway, amount);

/// @audit tokenAddresses()
115:              address gatewayToken = IAxelarGateway(gateway).tokenAddresses(tokenSymbol);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L30

File: contracts/deposit-service/DepositReceiver.sol

/// @audit delegatecall()
/// @audit receiverImplementation()
12:           (bool success, ) = IAxelarDepositService(msg.sender).receiverImplementation().delegatecall(delegateData);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol#L12

File: contracts/deposit-service/ReceiverImplementation.sol

/// @audit tokenAddresses()
25:           address tokenAddress = IAxelarGateway(gateway).tokenAddresses(symbol);

/// @audit refundToken()
27:           address refund = DepositBase(msg.sender).refundToken();

/// @audit balanceOf()
29:               _safeTransfer(refund, refundAddress, IERC20(refund).balanceOf(address(this)));

/// @audit balanceOf()
33:           uint256 amount = IERC20(tokenAddress).balanceOf(address(this));

/// @audit approve()
38:           IERC20(tokenAddress).approve(gateway, amount);

/// @audit refundToken()
49:           address refund = DepositBase(msg.sender).refundToken();

/// @audit balanceOf()
53:               _safeTransfer(refund, refundAddress, IERC20(refund).balanceOf(address(this)));

/// @audit approve()
64:           IERC20(wrappedTokenAddress).approve(gateway, amount);

/// @audit refundToken()
74:           address refund = DepositBase(msg.sender).refundToken();

/// @audit balanceOf()
76:               _safeTransfer(refund, refundAddress, IERC20(refund).balanceOf(address(this)));

/// @audit balanceOf()
80:           uint256 amount = IERC20(wrappedTokenAddress).balanceOf(address(this));

/// @audit withdraw()
85:           IWETH9(wrappedTokenAddress).withdraw(amount);

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L25

File: contracts/gas-service/AxelarGasService.sol

/// @audit balanceOf()
130:                  uint256 amount = IERC20(token).balanceOf(address(this));

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L130

[G‑03] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

There are 7 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

86        function _validateSignatures(
87            bytes32 messageHash,
88            address[] memory operators,
89            uint256[] memory weights,
90            uint256 threshold,
91:           bytes[] memory signatures

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

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L86-L91

File: contracts/AxelarGateway.sol

611:      function _setTokenDailyMintAmount(string memory symbol, uint256 amount) internal {

622:      function _setTokenAddress(string memory symbol, address tokenAddress) internal {

630       function _setContractCallApproved(
631           bytes32 commandId,
632           string memory sourceChain,
633           string memory sourceAddress,
634           address contractAddress,
635:          bytes32 payloadHash

640       function _setContractCallApprovedWithMint(
641           bytes32 commandId,
642           string memory sourceChain,
643           string memory sourceAddress,
644           address contractAddress,
645           bytes32 payloadHash,
646           string memory symbol,
647:          uint256 amount

655:      function _setImplementation(address newImplementation) internal {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L611

[G‑04] <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

  • storage arrays incur a Gwarmaccess (100 gas)
  • memory arrays use MLOAD (3 gas)
  • calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 7 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

17:           for (uint256 i; i < recentOperators.length; ++i) {

98:           for (uint256 i = 0; i < signatures.length; ++i) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L17

File: contracts/AxelarGateway.sol

207:          for (uint256 i = 0; i < symbols.length; i++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L207

File: contracts/deposit-service/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++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L114

File: contracts/gas-service/AxelarGasService.sol

123:          for (uint256 i; i < tokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L123

[G‑05] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

There are 12 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

17:           for (uint256 i; i < recentOperators.length; ++i) {

69:           for (uint256 i = 0; i < weightsLength; ++i) {

98:           for (uint256 i = 0; i < signatures.length; ++i) {

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

116:          for (uint256 i; i < accounts.length - 1; ++i) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L17

File: contracts/AxelarGateway.sol

195:          for (uint256 i; i < adminCount; ++i) {

207:          for (uint256 i = 0; i < symbols.length; i++) {

292:          for (uint256 i; i < commandsLength; ++i) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L195

File: contracts/deposit-service/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++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L114

File: contracts/gas-service/AxelarGasService.sol

123:          for (uint256 i; i < tokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L123

[G‑06] keccak256() should only need to be called on a specific string literal once

It should be saved to an immutable variable, and the variable used instead. If the hash is being used as a part of a function selector, the cast to bytes4 should also only be done once

There are 4 instances of this issue:

File: contracts/deposit-service/AxelarDepositServiceProxy.sol

9:            return keccak256('axelar-deposit-service');

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositServiceProxy.sol#L9

File: contracts/deposit-service/AxelarDepositService.sol

242:          return keccak256('axelar-deposit-service');

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L242

File: contracts/gas-service/AxelarGasServiceProxy.sol

10:           return keccak256('axelar-gas-service');

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasServiceProxy.sol#L10

File: contracts/gas-service/AxelarGasService.sol

181:          return keccak256('axelar-gas-service');

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L181

[G‑07] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 10 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

/// @audit validateProof(), transferOperatorship()
9:    contract AxelarAuthWeighted is Ownable, IAxelarAuthWeighted {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L9

File: contracts/AxelarGateway.sol

/// @audit sendToken(), callContract(), callContractWithToken(), deployToken(), mintToken(), burnToken(), approveContractCall(), approveContractCallWithMint(), transferOperatorship(), _unpackLegacyCommands()
15:   contract AxelarGateway is IAxelarGateway, AdminMultisigBase {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L15

File: contracts/deposit-service/AxelarDepositService.sol

/// @audit sendNative(), addressForTokenDeposit(), addressForNativeDeposit(), addressForNativeUnwrap(), sendTokenDeposit(), refundTokenDeposit(), sendNativeDeposit(), refundNativeDeposit(), nativeUnwrap(), refundNativeUnwrap(), contractId()
15:   contract AxelarDepositService is Upgradable, DepositBase, IAxelarDepositService {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L15

File: contracts/deposit-service/ReceiverImplementation.sol

/// @audit receiveAndSendToken(), receiveAndSendNative(), receiveAndUnwrapNative()
11:   contract ReceiverImplementation is DepositBase {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L11

File: contracts/gas-service/AxelarGasService.sol

/// @audit collectFees(), refund(), contractId()
10:   contract AxelarGasService is Upgradable, IAxelarGasService {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L10

File: contracts/interfaces/IAxelarAuth.sol

/// @audit validateProof(), transferOperatorship()
7:    interface IAxelarAuth is IOwnable {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuth.sol#L7

File: contracts/interfaces/IAxelarAuthWeighted.sol

/// @audit currentEpoch(), hashForEpoch(), epochForHash()
7:    interface IAxelarAuthWeighted is IAxelarAuth {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuthWeighted.sol#L7

File: contracts/interfaces/IAxelarDepositService.sol

/// @audit sendNative(), addressForTokenDeposit(), addressForNativeDeposit(), addressForNativeUnwrap(), sendTokenDeposit(), refundTokenDeposit(), sendNativeDeposit(), refundNativeDeposit(), nativeUnwrap(), refundNativeUnwrap(), receiverImplementation()
9:    interface IAxelarDepositService is IUpgradable, IDepositBase {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarDepositService.sol#L9

File: contracts/interfaces/IAxelarExecutable.sol

/// @audit execute(), executeWithToken()
7:    abstract contract IAxelarExecutable {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarExecutable.sol#L7

File: contracts/interfaces/IAxelarGasService.sol

/// @audit payGasForContractCall(), payGasForContractCallWithToken(), payNativeGasForContractCall(), payNativeGasForContractCallWithToken(), addGas(), addNativeGas(), collectFees(), refund()
8:    interface IAxelarGasService is IUpgradable {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L8

[G‑08] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

There are 5 instances of this issue:

File: contracts/AxelarGateway.sol

207:          for (uint256 i = 0; i < symbols.length; i++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L207

File: contracts/deposit-service/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++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L114

File: contracts/gas-service/AxelarGasService.sol

123:          for (uint256 i; i < tokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L123

[G‑09] Empty blocks should be removed or emit something

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}}). Empty receive()/fallback() payable functions that are not used, can be removed to save deployment gas.

There are 2 instances of this issue:

File: contracts/interfaces/IAxelarExecutable.sol

46:       ) internal virtual {}

54:       ) internal virtual {}

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarExecutable.sol#L46

[G‑10] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

There are 11 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

47:       function transferOperatorship(bytes calldata params) external onlyOwner {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L47

File: contracts/AxelarGateway.sol

204:      function setTokenDailyMintLimits(string[] calldata symbols, uint256[] calldata limits) external override onlyAdmin {

217       function upgrade(
218           address newImplementation,
219           bytes32 newImplementationCodeHash,
220           bytes calldata setupParams
221:      ) external override onlyAdmin {

331:      function deployToken(bytes calldata params, bytes32) external onlySelf {

367:      function mintToken(bytes calldata params, bytes32) external onlySelf {

373:      function burnToken(bytes calldata params, bytes32) external onlySelf {

397:      function approveContractCall(bytes calldata params, bytes32 commandId) external onlySelf {

411:      function approveContractCallWithMint(bytes calldata params, bytes32 commandId) external onlySelf {

437:      function transferOperatorship(bytes calldata newOperatorsData, bytes32) external onlySelf {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L204

File: contracts/gas-service/AxelarGasService.sol

120:      function collectFees(address payable receiver, address[] calldata tokens) external onlyOwner {

136       function refund(
137           address payable receiver,
138           address token,
139           uint256 amount
140:      ) external onlyOwner {

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L120

#0 - re1ro

2022-08-05T06:18:25Z

Dup #2 #3

#1 - GalloDaSballo

2022-08-25T01:28:08Z

[G‑01] Using calldata instead of memory for read-only arguments in external functions saves gas

60 for the array of bytes

[G‑02] Avoid contract existence checks by using solidity version 0.8.10 or later

100 gas per instance 2500

[G‑03] internal functions only called once can be inlined to save gas

20 per instance 140

[G‑04] <array>.length should not be looked up in every loop of a for-loop + G-05

Giving 300 consistently with rest of submissions

[G‑06] keccak256() should only need to be called on a specific string literal once

30 gas per instance 120

Rest is too opinionated for me :P

Great report as usual, would love to see a couple customized suggestion (packing or similar) and benchmarks, but still really good

3120 gas saved

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Ā© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter