Axelar Network v2 contest - ajtra'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: 19/75

Findings: 2

Award: $115.44

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Summary

Low

  1. L-1. Missing zero address validation
  2. L-2. Local variable shadowing
  3. L-3. Unused return
  4. L-4. Use a most recent solidity version

Non-Critical

  1. NC-1. Assembly usage
  2. NC-2. Conformance to Solidity naming conventions

Low

L-1. Missing zero address validation

Description

Detect missing zero address validation

Mitigation

Include check that the address is not zero

Lines in the code

constructor (ReceiverImplementation)

AxelarDepositService.sol#L18 AxelarDepositService.sol#L19

receiveAndSendToken (refundAddress.transfer)

ReceiverImplementation.sol#L17 ReceiverImplementation.sol#L23

receiveAndSendNative (refundAddress.transfer)

ReceiverImplementation.sol#L45 ReceiverImplementation.sol#L51

receiveAndUnwrapNative (refundAddress.transfer)

ReceiverImplementation.sol#L70 ReceiverImplementation.sol#L71 ReceiverImplementation.sol#L86

L-2. Local variable shadowing

Description

Detection of shadowing using local variables.

Mitigation

Rename the local variables that shadow another component.

Lines in the code

gateway

ReceiverImplementation.sol#12 DepositBase.sol#13 IDepositBase.sol#14

wrappedSymbol

ReceiverImplementation.sol#12 DepositBase.sol#13 IDepositBase.sol#14

L-3. Unused return

Description

There is return that are unusefull.

Mitigation

Remove unused return and include the rest code in else condition

Lines in the code

ReceiverImplementation.sol#L30 ReceiverImplementation.sol#L54

L-4. Use a most recent solidity version

Description

Actually the code use version 0.8.9

Mitigation

Update to use at least 0.8.10 to add more functionality like external calls skip contract existence checks if the external call has a return value

Non-Critical

NC-1. Assembly usage

Description

The use of assembly is error-prone and should be avoided.

Mitigation

Do not use evm assembly.

Lines in the code

DepositBase.sol#L54 DepositReceiver.sol#L17

NC-2. Conformance to Solidity naming conventions

Description

Solidity defines a naming convention that should be followed.

Mitigation

Follow the Solidity naming convention. Link

Lines in the code

AxelarGateway.sol#L447 AxelarGateway.sol#L45 AxelarGateway.sol#L46

#0 - GalloDaSballo

2022-08-28T20:35:28Z

L-1. Missing zero address validation

L

L-2. Local variable shadowing

R

L-3. Unused return

Disagree, they return early in a specific case

L-4. Use a most recent solidity version

NC

NC-1. Assembly usage

Disagree, you have to prove your statements here

NC-2. Conformance to Solidity naming conventions

Please tell the sponsor what they can do better instead of linking the Style Guide without context

1L 1R 1NC

Summary

  1. Post-increment/decrement cost more gas then pre-increment/decrement
  2. Call to KECCAK256 should use IMMUTABLE rather than constant
  3. Operatos <= or >= cost more gas than operators < or >
  4. != 0 is cheaper than >
  5. Variable1 = Variable1 + (-) Variable2 is cheaper in gas cost than variable1 += (-=) variable2.
  6. Use custom errors rather than REVERT()/REQUIRE() strings to save deployment gas
  7. Usage of uints/ints smaller than 32 Bytes (256 bits) incurs overhead
  8. Use a more recent version of solidity
  9. Initialize variables with default values are not needed
  10. Using bools for storage incurs overhead
  11. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()
  12. Optimize names to save gas
  13. Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate
  14. Calldata vs Memory
  15. Public function that could be declared external
  16. Unchecked arithmetic when it is not possible for them to overflow
  17. Empty blocks should be removed or emit something
  18. State variables that could be declared constant

Details

1. Post-increment/decrement cost more gas then pre-increment/decrement

Description

++var (--var) cost less gas than var++ (var--)

Lines in the code

AxelarGateway.sol#L207 AxelarGasService.sol#L123 AxelarDepositService.sol#L114 AxelarDepositService.sol#L168 AxelarDepositService.sol#L204

2. Call to KECCAK256 should use IMMUTABLE rather than constant

Description

Expressions for constant values such as a call to KECCAK256 should use IMMUTABLE rather than constant

Lines in the code

AxelarGateway.sol#L30 AxelarGateway.sol#L31 AxelarGateway.sol#L32 AxelarGateway.sol#L33 AxelarGateway.sol#L34 AxelarGateway.sol#L35 AxelarGateway.sol#L36 AxelarGateway.sol#L38 AxelarGateway.sol#L39 AxelarGateway.sol#L40 AxelarGateway.sol#L41 AxelarGateway.sol#L42 AxelarGateway.sol#L43

3. Operatos <= or >= cost more gas than operators < or >

Description

Change all <= / >= operators for < / > and remember to increse / decrese in consecuence to maintain the logic (example, a <= b for a < b + 1)

Lines in the code

AxelarAuthWeighted.sol#L36 AxelarAuthWeighted.sol#L107 AxelarAuthWeighted.sol#L117

4. != 0 is cheaper than >

Description

Replace all > 0 for != 0

Lines in the code

AxelarGateway.sol#L255 AxelarGateway.sol#L613 AxelarGasService.sol#L128 AxelarGasService.sol#L131 AxelarDepositService.sol#L165 ReceiverImplementation.sol#L23 ReceiverImplementation.sol#L51 ReceiverImplementation.sol#L71 AxelarAuthWeighted.sol#L76 AxelarAuthWeighted.sol#L76

5. Variable1 = Variable1 + (-) Variable2 is cheaper in gas cost than variable1 += (-=) variable2.

Description

Lines in the code

AxelarAuthWeighted.sol#L70 AxelarAuthWeighted.sol#L105

6. Use custom errors rather than REVERT()/REQUIRE() strings to save deployment gas

Description

Lines in the code

XC20Wrapper.sol#L55 XC20Wrapper.sol#L56 XC20Wrapper.sol#L57 XC20Wrapper.sol#L58 XC20Wrapper.sol#L61 XC20Wrapper.sol#L62 XC20Wrapper.sol#L68 XC20Wrapper.sol#L70 XC20Wrapper.sol#L78 XC20Wrapper.sol#L79 XC20Wrapper.sol#L84 XC20Wrapper.sol#L85 XC20Wrapper.sol#L86 XC20Wrapper.sol#L98 XC20Wrapper.sol#L111

7. Usage of uints/ints smaller than 32 Bytes (256 bits) incurs overhead

Description

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

Lines in the code

AxelarGateway.sol#L332 AxelarGateway.sol#L334 AxelarDepositService.sol#L225 AxelarDepositService.sol#L226 AxelarAuthWeighted.sol#L14

8. Use a more recent version of solidity

Description

Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

Lines in the code

AxelarGateway.sol#L3 IDepositBase.sol#L3 AxelarGasServiceProxy.sol#L3 AxelarGasService.sol#L3 IAxelarGasService.sol#L3 IAxelarAuth.sol#L3 IAxelarAuthWeighted.sol#L3 IAxelarDepositService.sol#L3 DepositReceiver.sol#L3 AxelarDepositService.sol#L3 AxelarDepositServiceProxy.sol#L3 DepositBase.sol#L3 ReceiverImplementation.sol#L3 AxelarAuthWeighted.sol#L3 XC20Wrapper.sol#L3

9. Initialize variables with default values are not needed

Description

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address,...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

AxelarGateway.sol#L207 AxelarAuthWeighted.sol#L69 AxelarAuthWeighted.sol#L98

Lines in the code

10. Using bools for storage incurs overhead

Description

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past

Lines in the code

AxelarGateway.sol#L101 AxelarGateway.sol#L113 AxelarGateway.sol#L125 AxelarGateway.sol#L138 AxelarGateway.sol#L160 AxelarGateway.sol#L172 AxelarGateway.sol#L176 AxelarGateway.sol#L229 AxelarGateway.sol#L268 AxelarGateway.sol#L320 AxelarGateway.sol#L344 AxelarGateway.sol#L383 AxelarGateway.sol#L388 AxelarGateway.sol#L460 AxelarGateway.sol#L461 AxelarGateway.sol#L462 AxelarGateway.sol#L477 AxelarGateway.sol#L496 AxelarGateway.sol#L626 AxelarGasService.sol#L158 AxelarGasService.sol#L159 AxelarGasService.sol#L172 AxelarGasService.sol#L175 IAxelarAuth.sol#L8 DepositReceiver.sol#L12 DepositBase.sol#L71 DepositBase.sol#L72 AxelarAuthWeighted.sol#L26 AxelarAuthWeighted.sol#L115 XC20Wrapper.sol#L95 XC20Wrapper.sol#L96 XC20Wrapper.sol#L106 XC20Wrapper.sol#L109

11. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()

Description

Try to use ABI.ENCODEPACKED() to save gas.

Lines in the code

AxelarGateway.sol#L566 AxelarGateway.sol#L580 AxelarDepositService.sol#L233 AxelarAuthWeighted.sol#L32

12. Optimize names to save gas

Description

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. Link

13. Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Description

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

Lines in the code

XC20Wrapper.sol#L20 XC20Wrapper.sol#L21

14. Calldata vs Memory

Description

Use calldata instead of memory in a function parameter when you are only to read the data can save gas by storing it in calldata when the function is external

Lines in the code

AxelarGateway.sol#L447

15. Public function that could be declared external

Description

public functions that are never called by the contract should be declared external to save gas

Lines in the code

AxelarDepositService.sol#L241 DepositBase.sol#L41

16. Unchecked arithmetic when it is not possible for them to overflow

Description

The default β€œchecked” behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected. if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas.

For all for-loops in the code it is possible to change as the following example.

for (uint256 i;i < X;){ -- code -- unchecked { ++i; } }

Lines in the code

AxelarGateway.sol#L195 AxelarGateway.sol#L207 AxelarGateway.sol#L292 AxelarGasService.sol#L123 AxelarDepositService.sol#L114 AxelarDepositService.sol#L168 AxelarDepositService.sol#L204 AxelarAuthWeighted.sol#L17 AxelarAuthWeighted.sol#L69 AxelarAuthWeighted.sol#L98 AxelarAuthWeighted.sol#L101 AxelarAuthWeighted.sol#L116

17. Empty blocks should be removed or emit something

Description

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{...}})

Lines in the code

DepositReceiver.sol#L29

18. State variables that could be declared constant

Description

Constant state variables should be declared constant to save gas. Add the constant attributes to state variables that never change.

Lines in the code

DepositBase.sol#L19

#0 - re1ro

2022-08-05T09:58:10Z

Dup #2 #3 #7 #122

#1 - GalloDaSballo

2022-08-20T18:57:02Z

  1. Call to KECCAK256 should use IMMUTABLE rather than constant Disputed -> https://twitter.com/GalloDaSballo/status/1543729080926871557

Less than 500 gas saved.

The advice on upgrading solidity is technically correct, however the submission doesn't list the external calls so I can't quantify it

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