Axelar Network - Sathish9098's results

Decentralized interoperability network.

General Information

Platform: Code4rena

Start Date: 12/07/2023

Pot Size: $80,000 USDC

Total HM: 11

Participants: 47

Period: 9 days

Judge: berndartmueller

Total Solo HM: 1

Id: 260

League: ETH

Axelar Network

Findings Distribution

Researcher Performance

Rank: 11/47

Findings: 3

Award: $1,117.45

QA:
grade-b
Gas:
grade-a
Analysis:
grade-a

🌟 Selected for report: 1

🚀 Solo Findings: 0

Findings Information

Labels

bug
grade-b
QA (Quality Assurance)
edited-by-warden
Q-15

Awards

43.3267 USDC - $43.33

External Links

LOW FINDINGS

[L-1] _getTokenMintLimitKey,_getTokenTypeKey,_getTokenAddressKey,_getIsCommandExecutedKey functions not prevent the duplicate symbol values

Impact

If the functions _getTokenMintLimitKey, _getTokenTypeKey, _getTokenAddressKey, and _getIsCommandExecutedKey do not incorporate any uniqueness, and symbol is the only varying parameter, then there will be hash collisions for duplicate symbol values.

FILE: 2023-07-axelar/contracts/cgp/AxelarGateway.sol

 function _getTokenMintLimitKey(string memory symbol) internal pure returns (bytes32) {
        return keccak256(abi.encodePacked(PREFIX_TOKEN_MINT_LIMIT, symbol));
    }

  function _getTokenTypeKey(string memory symbol) internal pure returns (bytes32) {
        return keccak256(abi.encodePacked(PREFIX_TOKEN_TYPE, symbol));
    }

    function _getTokenAddressKey(string memory symbol) internal pure returns (bytes32) {
        return keccak256(abi.encodePacked(PREFIX_TOKEN_ADDRESS, symbol));
    }

    function _getIsCommandExecutedKey(bytes32 commandId) internal pure returns (bytes32) {
        return keccak256(abi.encodePacked(PREFIX_COMMAND_EXECUTED, commandId));
    }

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/AxelarGateway.sol#L556-L558

Unique nonce or timestamp in the abi.encodePacked process to ensure that each call produces a unique hash


uint256 private nonce;

function _getTokenMintLimitKey(string memory symbol) private returns (bytes32) {
    return keccak256(abi.encodePacked(PREFIX_TOKEN_MINT_LIMIT, symbol, nonce++));
}

[L-2] Lack of pause state Check in setFlowLimit() function Poses security risk in contract

Impact

The problem is the services paused by onlyOwner . But FlowLimit set by onlyOperatorthis will lead to unintended consequences.

Allowing setFlowLimit() to execute when the contract is paused can lead to unintended consequences and may compromise the expected behavior of the paused contract. Depending on how the contract utilizes the FlowLimit, this can introduce potential security vulnerabilities or operational issues.

POC

FILE: 2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

 function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator {
        uint256 length = tokenIds.length;
        if (length != flowLimits.length) revert LengthMismatch();
        for (uint256 i; i < length; ++i) {
            ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenIds[i]));
            tokenManager.setFlowLimit(flowLimits[i]);
        }
    }

Incorporate the notPaused modifier in the setFlowLimit() function

[L-3] OnlyOwner allows malicious address as a trusted address

Impact

he function allows the contract owner to add any address as a trusted address, regardless of whether the address is actually controlled by the interchain token service. This could allow an attacker to gain control of the contract by adding a malicious address as a trusted address.

POC

FILE: 2023-07-axelar/contracts/its/remote-address-validator/RemoteAddressValidator.sol

 function addTrustedAddress(string memory chain, string memory addr) public onlyOwner {
        if (bytes(chain).length == 0) revert ZeroStringLength();
        if (bytes(addr).length == 0) revert ZeroStringLength();
        remoteAddressHashes[chain] = keccak256(bytes(_lowerCase(addr)));
        remoteAddresses[chain] = addr;
        emit TrustedAddressAdded(chain, addr);
    }

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L83-L89

addTrustedAddress function could be modified to require that the address to be added be signed by the interchain token service. This would ensure that only trusted addresses are added to the contract.

[L-4] Avoid Hardcoded Values

Impact

Two constants PREFIX_EXPRESS_RECEIVE_TOKEN and PREFIX_EXPRESS_RECEIVE_TOKEN_WITH_DATA are hardcoded. This means that anyone who knows the contract code will know what the prefixes are. This could make it easier for attackers to forge transactions that look like they were sent from the Axelar gateway.

codehash != 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 hardcoded value check is not appreciated . This will cause problem in future .

Possible Problems:

  • Hardcoded values can make your contract more vulnerable to attack.
  • Hardcoded values can make your contract more difficult to update.
  • Hardcoded values can make your contract less flexible.

POC

FILE: Breadcrumbs2023-07-axelar/contracts/cgp/AxelarGateway.sol

509: return codehash != bytes32(0) && codehash != 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470; 

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/AxelarGateway.sol#L35

FILE: 2023-07-axelar/contracts/its/utils/ExpressCallHandler.sol

 uint256 internal constant PREFIX_EXPRESS_RECEIVE_TOKEN = 0x67c7b41c1cb0375e36084c4ec399d005168e83425fa471b9224f6115af865619;
    // uint256(keccak256('prefix-express-give-token-with-data'));
    uint256 internal constant PREFIX_EXPRESS_RECEIVE_TOKEN_WITH_DATA = 0x3e607cc12a253b1d9f677a03d298ad869a90a8ba4bd0fb5739e7d79db7cdeaad;

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/utils/ExpressCallHandler.sol#L14-L16

It would be better to generate the prefixes dynamically and in lowercase. This would make the contract more secure and more flexible

[L-5] sstore instruction is a mutable instruction, which means that it can be overwritten by another transaction

Impact

The _setExpressReceiveToken function uses the sstore assembly instruction to store the value of the expressCaller variable in the contract storage. The sstore instruction is a mutable instruction, which means that it can be overwritten by another transaction. This means that if an attacker were to send a transaction that overwrites the value of the expressCaller variable, they could effectively steal the tokens that were sent to the destination address

POC

FILE: 2023-07-axelar/contracts/its/utils/ExpressCallHandler.sol

 if (prevExpressCaller != address(0)) revert AlreadyExpressCalled();
        assembly {
            sstore(slot, expressCaller)
        }

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/utils/ExpressCallHandler.sol#L91-L94

_setExpressReceiveToken function, the sstore instruction should be used with the lock modifier. The lock modifier prevents the value of the expressCaller variable from being overwritten by another transaction

[L-6] Use IERC20.allowance() function to check the allowance instead of a custom mapping

Impact

Custom mappings consume storage on the Ethereum blockchain. If you have a large number of users or tokens, maintaining allowances in a mapping can lead to higher storage costs. Multiple concurrent transactions attempting to update allowances, there may be potential concurrency issues or race conditions.When using a custom mapping, you need to ensure that the allowances are synchronized and consistent between different parts of the contract

POC

FILE: 2023-07-axelar/contracts/its/interchain-token/InterchainToken.sol

55: uint256 allowance_ = allowance[sender][address(tokenManager)];

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token/InterchainToken.sol#L55

Use IERC20.allowance() function for handling token allowances.

[L-7] Use safeIncreaseAllowance()/safeDecreaseAllowance() instead of normal approve() function

Impact

The safeIncreaseAllowance() and safeDecreaseAllowance() functions are more secure than the approve() function. The approve() function allows an account to approve another account to transfer an unlimited amount of tokens on their behalf. This could be exploited by an attacker to transfer more tokens than the sender intended.

POC

FILE: Breadcrumbs2023-07-axelar/contracts/its/interchain-token/InterchainToken.sol

61: _approve(sender, address(tokenManager), allowance_ + amount);

100: _approve(sender, address(tokenManager), allowance_ + amount);

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token/InterchainToken.sol#L100

safeIncreaseAllowance() and safeDecreaseAllowance() functions prevent this by ensuring that the amount of tokens that an account is approved to transfer cannot be increased or decreased by more than the current allowance.

[L-8] Revert on Zero Value Transfers

Impact

Some tokens revert when transferring a zero value amount. Many ERC-20 and ERC-721 token contracts implement a safeguard that reverts transactions which attempt to transfer tokens with zero amount. This is because such transfers are often the result of programming errors. The OpenZeppelin library, a popular choice for implementing these standards, includes this safeguard. For token contract developers who want to avoid unintentional transfers with zero amount, it's good practice to include a condition that reverts the transaction if the amount is zero.

POC


451: SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L439-L451


482: SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L467-L482


48: SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), amount);

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/token-manager/implementations/TokenManagerLockUnlock.sol#L44-L48


82: SafeTokenTransferFrom.safeTransferFrom(token, from, liquidityPool_, amount);

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/token-manager/implementations/TokenManagerLiquidityPool.sol#L77-L82

98:  SafeTokenTransferFrom.safeTransferFrom(token, liquidityPool(), to, amount);

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/token-manager/implementations/TokenManagerLiquidityPool.sol#L94-L98

Make sure amount > 0 before calling transfer function

[L-9] Revert on Zero Value Approvals

Impact

Some tokens (e.g. BNB) revert when approving a zero value amount (i.e. a call to approve(address, 0)). Integrators may need to add special cases to handle this logic if working with such a token.

POC


61: _approve(sender, address(tokenManager), allowance_ + amount);

100: _approve(sender, address(tokenManager), allowance_ + amount);

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token/InterchainToken.sol#L100

[L-10] registerCanonicalToken() Not checking whether the tokenaddress already managed by a token manager

Impact

If two different token managers are managing the same token address, there is a risk of conflicting behavior. For example, one token manager might allow transfers while another token manager might disallow transfers. This could lead to errors or unexpected behavior for users.

POC

FILE: 2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

 function registerCanonicalToken(address tokenAddress) external payable notPaused returns (bytes32 tokenId) {
        (, string memory tokenSymbol, ) = _validateToken(tokenAddress);
        if (gateway.tokenAddresses(tokenSymbol) == tokenAddress) revert GatewayToken();
        tokenId = getCanonicalTokenId(tokenAddress);
        _deployTokenManager(tokenId, TokenManagerType.LOCK_UNLOCK, abi.encode(address(this).toBytes(), tokenAddress));
    }

Add isTokenManaged() function to check the token address existence


function registerCanonicalToken(address tokenAddress) external payable notPaused returns (bytes32 tokenId) {
(, string memory tokenSymbol, ) = _validateToken(tokenAddress);
if (gateway.tokenAddresses(tokenSymbol) == tokenAddress) revert GatewayToken();
+ if (isTokenManaged(tokenAddress)) revert TokenAlreadyManaged();
tokenId = getCanonicalTokenId(tokenAddress);
_deployTokenManager(tokenId, TokenManagerType.LOCK_UNLOCK, abi.encode(address(this).toBytes(), tokenAddress));

}

[L-11] Unused/empty receive()/fallback() function

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

POC

FILE: 2023-07-axelar/contracts/cgp/governance/InterchainGovernance.sol

168: receive() external payable {}

[L-12] _approve() doesn’t check return value

Impact

if the approve() function fails, the caller will not be notified of the failure

POC


61: _approve(sender, address(tokenManager), allowance_ + amount);

100: _approve(sender, address(tokenManager), allowance_ + amount);

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token/InterchainToken.sol#L100

Add the control to check _approve() status

[L-13] Eth that is accidentally sent to a receive() function cannot be withdrawn

Impact

The receive() function is used to handle incoming ether (native currency) transfers to a contract. However, when tokens (other than the native currency) are accidentally sent to the receive() function, they are effectively trapped within the contract, and there is no built-in mechanism to withdraw or recover them.

POC

FILE: 2023-07-axelar/contracts/cgp/governance/InterchainGovernance.sol

168: receive() external payable {}

Add Recovery mechanism to recover Eth sent accidently to the contract

[L-14] Lack of access control in executeProposal() function

Impact

There is a lack of access control in the executeProposal() function of the Interchain Governance contract. Without access control, any external account can call this function and execute governance proposals, which may lead to unauthorized or malicious actions being taken

POC

FILE: 2023-07-axelar/contracts/cgp/governance/InterchainGovernance.sol

function executeProposal(
        address target,
        bytes calldata callData,
        uint256 nativeValue
    ) external payable {
        bytes32 proposalHash = keccak256(abi.encodePacked(target, callData, nativeValue));

        _finalizeTimeLock(proposalHash);
        _call(target, callData, nativeValue);

        emit ProposalExecuted(proposalHash, target, callData, nativeValue, block.timestamp);
    }

Add modifiers like onlyOwner to execute the proposals

[L-15] sourceChain and sourceAddress string comparison is vulnerable to timing attacks

Impact

In the _execute function, the contract uses string comparison to verify if the provided sourceChain and sourceAddress match the expected governance chain and address. This method is vulnerable to timing attacks, where an attacker could exploit the time taken for the comparison to determine partial information about the comparison result.

POC

FILE: 2023-07-axelar/contracts/cgp/governance/InterchainGovernance.sol

 if (keccak256(bytes(sourceChain)) != governanceChainHash || keccak256(bytes(sourceAddress)) != governanceAddressHash)
            revert NotGovernance();

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/governance/InterchainGovernance.sol#L92-L93

Use bytes32 for governanceChainHash and governanceAddressHash, and use == for direct comparison

[L-16] ``InterchainProposalSender.sol contract lacks nonce handling mechanism to prevent replay attacks

Impact

The sendProposal function in the InterchainProposalSender contract can be susceptible to replay attacks without a nonce mechanism because it allows users to broadcast the same proposal multiple times.

leading to the following issues:

  • Double Execution
  • Unintended Cost
  • Invalid Replays

POC


function sendProposals(InterchainCalls.InterchainCall[] calldata interchainCalls) external payable override {
        // revert if the sum of given fees are not equal to the msg.value
        revertIfInvalidFee(interchainCalls);

        for (uint256 i = 0; i < interchainCalls.length; ) {
            _sendProposal(interchainCalls[i]);
            unchecked {
                ++i;
            }
        }
    }

Add nonce mechanism to sendProposal function

[L-17] Contract RemoteAddressValidator.sol is inherited ``upgradable` but not actually ungradable contract

Impact

The absence of an initialize function suggests that the contract is not intended to be upgradable using a proxy contract. Instead, it follows a traditional deployment approach, where the constructor is used to set up the initial state of the contract. Once deployed, the state and behavior of the contract are immutable, and any changes or updates would require deploying a new instance of the contract.

POC

FILE: 2023-07-axelar/contracts/its/remote-address-validator/RemoteAddressValidator.sol

12: contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L12

NON CRITICAL FINDINGS

[NC-1] Events is missing indexed fields

Index event fields make the field more quickly accessible to off-chain.

Each event should use three indexed fields if there are three or more fields.

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/interfaces/IAxelarServiceGovernance.sol#L15-L17

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/interfaces/IMultisigBase.sol#L22

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interfaces/IInterchainTokenService.sol#L32-L76

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interfaces/IExpressCallHandler.sol#L9-L43

[NC-2] Shorter the inheritance list

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L37-L44

#0 - c4-judge

2023-09-08T11:23:34Z

berndartmueller marked the issue as grade-b

Awards

246.6663 USDC - $246.67

Labels

bug
G (Gas Optimization)
grade-a
high quality report
selected for report
edited-by-warden
G-21

External Links

GAS OPTIMIZATIONS

GAS COUNTIssuesInstancesGas Saved
[G-1]Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate121018
[G-2]Using calldata instead of memory for read-only arguments in external functions saves gas82820
[G-3]Avoiding the overhead of bool storage6100600
[G-4]Avoid contract existence checks by using low level calls373700
[G-5]Use calldata pointer Saves more gas than memory pointer2600 GAS (Per Iteration)
[G-6]IF’s/require() statements that check input arguments should be at the top of the function2300
[G-7]Functions guaranteed to revert when called by normal users can be marked payable11231
[G-8]Optimize names to save gas27-
[G-9]Default value initialization480
[G-10]Use constants instead of type(uintX).max791
[G-11]Splitting require()/if() statements that use && saves gas452
[G-12]Caching global variables is more expensive than using the actual variable (use msg.sender instead of caching it)10-

[G-1] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

We can combine multiple mappings below into structs. We can then pack the structs by modifying the uint type for the values. This will result in cheaper storage reads since multiple mappings are accessed in functions and those values are now occupying the same storage slot, meaning the slot will become warm after the first SLOAD. In addition, when writing to and reading from the struct values we will avoid a Gsset (20000 gas) and Gcoldsload (2100 gas), since multiple struct values are now occupying the same slot.

RemoteAddressValidator.sol: struct can be used for remoteAddressHashes,remoteAddresses since they are using same string as key. Also both mapping always used together in a same functions like addTrustedAddress(), removeTrustedAddress() So struct is more efficient than mapping. As per gas tests this will save 21018 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L15-L16

FILE: 2023-07-axelar/contracts/its/remote-address-validator/RemoteAddressValidator.sol

- 15: mapping(string => bytes32) public remoteAddressHashes;
- 16: mapping(string => string) public remoteAddresses;

+ struct RemoteAddressData {
+    bytes32 addressHash;
+    string addressString;
+ }

+ mapping(string => RemoteAddressData) public remoteAddressData;
FILE: Breadcrumbs2023-07-axelar/contracts/its/remote-address-validator/RemoteAddressValidator.sol

15: mapping(string => bytes32) public remoteAddressHashes;
16: mapping(string => string) public remoteAddresses;

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

Saves 2820 GAS, 8 Instances

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

InterchainTokenService.sol: distributor,operator can be calldata instead of memory: Saves 564 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L422

FILE: 2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

417: function deployAndRegisterRemoteStandardizedToken(
418:        bytes32 salt,
419:        string calldata name,
420:        string calldata symbol,
421:        uint8 decimals,
- 422:        bytes memory distributor,
+ 422:        bytes calldata distributor,
- 423:        bytes memory operator,
+ 423:        bytes calldata operator,
424:        string calldata destinationChain,
425:        uint256 gasValue
426:    ) external payable notPaused {

InterchainTokenService.sol: sourceChain,sourceAddress,destinationAddress can be calldata instead of memory: Saves 846 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L467

FILE: 2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

467:   function expressReceiveTokenWithData(
468:        bytes32 tokenId,
- 469:        string memory sourceChain,
+ 469:        string calldata sourceChain,
- 470:        bytes memory sourceAddress,
+ 470:        bytes calldata sourceAddress,
471:        address destinationAddress,
472:        uint256 amount,
473:        bytes calldata data,
474:        bytes32 commandId
475:    ) external {


- 506:   bytes memory destinationAddress,
+ 506:   bytes calldata destinationAddress,

MultisigBase.sol: newAccounts can be calldata instead of memory: Saves 282 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/auth/MultisigBase.sol#L142-L144

FILE: 2023-07-axelar/contracts/cgp/auth/MultisigBase.sol

- 142: function rotateSigners(address[] memory newAccounts, uint256 newThreshold) external virtual onlySigners {
+ 142: function rotateSigners(address[] calldata newAccounts, uint256 newThreshold) external virtual onlySigners {
143:        _rotateSigners(newAccounts, newThreshold);
144:    }

InterchainProposalSender.sol: destinationChain,destinationContract can be calldata instead of memory: Saves 564 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/interchain-governance-executor/InterchainProposalSender.sol#L81

FILE: 2023-07-axelar/contracts/interchain-governance-executor/InterchainProposalSender.sol

80: function sendProposal(
- 81:        string memory destinationChain,
+ 81:        string calldata destinationChain,
- 82:        string memory destinationContract,
+ 82:        string calldata destinationContract,
83:        InterchainCalls.Call[] calldata calls
84:    ) external payable override {

ConstAddressDeployer.sol: bytecode,bytecode can be calldata instead of memory: Saves 564 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/gmp-sdk/deploy/ConstAddressDeployer.sol#L24

FILE: Breadcrumbs2023-07-axelar/contracts/gmp-sdk/deploy/ConstAddressDeployer.sol

- 24: function deploy(bytes memory bytecode, bytes32 salt) external returns (address deployedAddress_) {
+ 24: function deploy(bytes calldata bytecode, bytes32 salt) external returns (address deployedAddress_) {


42: function deployAndInit(
- 43:         bytes memory bytecode,
+ 43:         bytes calldata bytecode,
44:        bytes32 salt,
45:        bytes calldata init
46:    ) external returns (address deployedAddress_) {

[G-3] Avoiding the overhead of bool storage

Saves 120600 GAS, 6 Instances

// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false to true, after having been true in the past

FILE: 2023-07-axelar/contracts/cgp/auth/MultisigBase.sol

15: mapping(address => bool) hasVoted;
21: mapping(address => bool) isSigner;

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/auth/MultisigBase.sol#L15

FILE: 2023-07-axelar/contracts/cgp/governance/AxelarServiceGovernance.sol

22: mapping(bytes32 => bool) public multisigApprovals;

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/governance/AxelarServiceGovernance.sol#L22

FILE: 2023-07-axelar/contracts/interchain-governance-executor/InterchainProposalExecutor.sol

24: mapping(string => mapping(address => bool)) public whitelistedCallers;
27: mapping(string => mapping(address => bool)) public whitelistedSenders;

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L24

FILE: 2023-07-axelar/contracts/its/remote-address-validator/RemoteAddressValidator.sol

19: mapping(string => bool) public supportedByGateway;

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L19

[G-4] Avoid contract existence checks by using low level calls

Saves 3700 GAS, 37 Instances

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low level calls never check for contract existence

FILE: Breadcrumbs2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

102: deployer = ITokenManagerDeployer(tokenManagerDeployer_).deployer();

162: tokenManagerAddress = deployer.deployedAddress(address(this), tokenId);

172: if (ITokenManagerProxy(tokenManagerAddress).tokenId() != tokenId) revert TokenManagerDoesNotExist(tokenId);

182: tokenAddress = ITokenManager(tokenManagerAddress).tokenAddress();

193: tokenAddress = deployer.deployedAddress(address(this), tokenId);

277: flowLimit = tokenManager.getFlowLimit();

287: flowOutAmount = tokenManager.getFlowOutAmount();

297: flowInAmount = tokenManager.getFlowInAmount();

311: if (gateway.tokenAddresses(tokenSymbol) == tokenAddress) revert GatewayToken();

331: tokenAddress = ITokenManager(tokenAddress).tokenAddress();

445: if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);

476: if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);

480: IERC20 token = IERC20(tokenManager.tokenAddress());

539: tokenManager.setFlowLimit(flowLimits[i]);

566:  if (ITokenManager(implementation).implementationType() != uint256(tokenManagerType)) revert InvalidTokenManagerImplementation();

610: amount = tokenManager.giveToken(destinationAddress, amount);

653: amount = tokenManager.giveToken(expressCaller, amount);

657: amount = tokenManager.giveToken(destinationAddress, amount);

658: IInterchainTokenExpressExecutable(destinationAddress).executeWithInterchainToken(sourceChain, sourceAddress, data, tokenId, amount);

713: string memory destinationAddress = remoteAddressValidator.getRemoteAddress(destinationChain);

723: gateway.callContract(destinationChain, destinationAddress, payload);

735:  name = token.name();

736: symbol = token.symbol();

737: decimals = token.decimals();

888: IInterchainTokenExpressExecutable(destinationAddress).expressExecuteWithInterchainToken(
            sourceChain,
            sourceAddress,
            data,
            tokenId,
            amount
        );

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L102

FILE: Breadcrumbs2023-07-axelar/contracts/cgp/AxelarGateway.sol

287: if (AxelarGateway(newImplementation).contractId() != contractId()) revert InvalidImplementation();

317: IAxelarAuth(AUTH_MODULE).transferOperatorship(newOperatorsData);

329: bool allowOperatorshipTransfer = IAxelarAuth(AUTH_MODULE).validateProof(messageHash, proof);

449: depositHandler.destroy(address(this)

496: IAxelarAuth(AUTH_MODULE).transferOperatorship(newOperatorsData);

524: IERC20(tokenAddress).safeTransfer(account, amount);

526: IBurnableMintableCappedERC20(tokenAddress).mint(account, amount);

543: IERC20(tokenAddress).safeTransferFrom(sender, address(this), amount);

545: IERC20(tokenAddress).safeCall(abi.encodeWithSelector(IBurnableMintableCappedERC20.burnFrom.selector, sender, amount));

547: IERC20(tokenAddress).safeTransferFrom(sender, IBurnableMintableCappedERC20(tokenAddress).depositAddress(bytes32(0)), amount);
FILE: 2023-07-axelar/contracts/interchain-governance-executor/InterchainProposalSender.sol

101: gateway.callContract(interchainCall.destinationChain, interchainCall.destinationContract, payload);

[G-5] Use Calldata pointer Saves more gas than memory pointer

Saves 600 GAS, per Loop Iterations

Calling calldata instead of memory in the loop you have shown will save gas. This is because calldata is a read-only data structure, which means that it does not have to be copied into memory each time it is accessed.

InterchainProposalExecutor.sol: Use ```calldatainstead ofmemory: Saves250-300 GAS`` Per iteration

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L75

call value is not changed any where inside the loop. So calldata is more efficient than memory to save gas

FILE: Breadcrumbs2023-07-axelar/contracts/interchain-governance-executor/InterchainProposalExecutor.sol

74: for (uint256 i = 0; i < calls.length; i++) {
- 75:            InterchainCalls.Call memory call = calls[i];
+ 75:            InterchainCalls.Call calldata call = calls[i];
76:            (bool success, bytes memory result) = call.target.call{ value: call.value }(call.callData);
77:
78:            if (!success) {
79:                _onTargetExecutionFailed(call, result);
80:            } else {
81:                _onTargetExecuted(call, result);
82:            }

AxelarGateway.sol: Use ```calldatainstead ofmemory: Saves250-300 GAS`` Per iteration

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/AxelarGateway.sol#L270-L277

symbol value is not changed any where inside the loop. So calldata is more efficient than memory to save gas

FILE: 2023-07-axelar/contracts/cgp/AxelarGateway.sol

270: for (uint256 i; i < length; ++i) {
- 271:            string memory symbol = symbols[i];
+ 271:            string calldata symbol = symbols[i];
272:            uint256 limit = limits[i];
273:
274:            if (tokenAddresses(symbol) == address(0)) revert TokenDoesNotExist(symbol);
275:
276:            _setTokenMintLimit(symbol, limit);
277:        }

[G-6] IF’s/require() statements that check input arguments should be at the top of the function

Saves 300 GAS, 2 Instance

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas) in a function that may ultimately revert in the unhappy case.

Cheaper to check the function parameter before making check . Saves 200- 300 GAS

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/auth/MultisigBase.sol#L172-L173

FILE: 2023-07-axelar/contracts/cgp/auth/MultisigBase.sol

169:  address account = newAccounts[i];
170:
171: // Check that the account wasn't already set as a signer for this epoch.
+ 273: if (account == address(0)) revert InvalidSigners();
- 172: if (signers.isSigner[account]) revert DuplicateSigner(account);
- 273: if (account == address(0)) revert InvalidSigners();
+ 172: if (signers.isSigner[account]) revert DuplicateSigner(account);

tokenAddresses(symbol) == address(0) should be checked before limit to avoid unnecessary variable creation. After variable creation if any fails its waste of gas

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/AxelarGateway.sol#L271-L274

FILE: 2023-07-axelar/contracts/cgp/AxelarGateway.sol

271: string memory symbol = symbols[i];
+ 274: if (tokenAddresses(symbol) == address(0)) revert TokenDoesNotExist(symbol);
272: uint256 limit = limits[i];
273:
- 274: if (tokenAddresses(symbol) == address(0)) revert TokenDoesNotExist(symbol);

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

Saves 231 GAS, 11 Instance

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

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L96-L96

92:     function setWhitelistedProposalCaller(
93:         string calldata sourceChain,
94:         address sourceCaller,
95:         bool whitelisted
96:     ) external override onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L111-L111


107:     function setWhitelistedProposalSender(
108:         string calldata sourceChain,
109:         address sourceSender,
110:         bool whitelisted
111:     ) external override onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/gmp-sdk/upgradable/Upgradable.sol#L56-L56


52:     function upgrade(
53:         address newImplementation,
54:         bytes32 newImplementationCodeHash,
55:         bytes calldata params
56:     ) external override onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L547-L547

547:     function setPaused(bool paused) external onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L83-L83


83:     function addTrustedAddress(string memory chain, string memory addr) public onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L95-L95


95:     function removeTrustedAddress(string calldata chain) external onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L106-L106

106:     function addGatewaySupportedChains(string[] calldata chainNames) external onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L119-L119

119:     function removeGatewaySupportedChains(string[] calldata chainNames) external onlyOwner 
https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L343-L345 ```solidity 343: function deployCustomTokenManager( 344: bytes32 salt, 345: TokenManagerType tokenManagerType, 346: bytes memory params 347: ) public payable notPaused returns (bytes32 tokenId)

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L365-L368


365:     function deployRemoteCustomTokenManager( 
366:         bytes32 salt,
367:         string calldata destinationChain,
368:         TokenManagerType tokenManagerType, 
369:         bytes calldata params,
370:         uint256 gasValue
371:     ) external payable notPaused returns (bytes32 tokenId) 

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L509-L509

502:     function transmitSendToken(
503:         bytes32 tokenId,
504:         address sourceAddress,
505:         string calldata destinationChain,
506:         bytes memory destinationAddress,
507:         uint256 amount,
508:         bytes calldata metadata
509:     ) external payable onlyTokenManager(tokenId) notPaused  

[G-8] Optimize names to save gas

Saves 27 Instances

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


///@audit getProposalEta(),executeProposal(),
FILE: 2023-07-axelar/contracts/cgp/governance/InterchainGovernance.sol

///@audit executeMultisigProposal(),
FILE: 2023-07-axelar/contracts/cgp/governance/AxelarServiceGovernance.sol

///@audit getChainName(),getTokenManagerAddress(),getValidTokenManagerAddress(),getTokenAddress(),getStandardizedTokenAddress(),getCanonicalTokenId(),getCustomTokenId(),getImplementation(),getParamsLockUnlock(),getParamsMintBurn(),getParamsLiquidityPool(),getFlowLimit(),getFlowOutAmount(),getFlowInAmount(),registerCanonicalToken(),deployRemoteCanonicalToken(),deployCustomTokenManager(),deployRemoteCustomTokenManager(),deployAndRegisterStandardizedToken(),deployAndRegisterRemoteStandardizedToken(),expressReceiveToken(),expressReceiveTokenWithData(),transmitSendToken(),setFlowLimit(),
FILE: 2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

[G-9] Default value initialization

Saves 80 GAS,4 Instance

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Saves 15-20 GAS per instance

FILE: 2023-07-axelar/contracts/interchain-governance-executor/InterchainProposalSender.sol

- 63: for (uint256 i = 0; i < interchainCalls.length; ) {
+ 63: for (uint256 i; i < interchainCalls.length; ) {

- 105: uint256 totalGas = 0;
+ 105: uint256 totalGas ;

- 106: for (uint256 i = 0; i < interchainCalls.length; ) {
+ 106: for (uint256 i ; i < interchainCalls.length; ) {

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/interchain-governance-executor/InterchainProposalSender.sol#L63

FILE: 2023-07-axelar/contracts/interchain-governance-executor/InterchainProposalExecutor.sol

- 74: for (uint256 i = 0; i < calls.length; i++) {
+ 74: for (uint256 i ; i < calls.length; i++) {

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L74

[G-10] Use constants instead of type(uintX).max

Saves 91 GAS,7 Instance

Using constants instead of type(uintX).max saves gas in Solidity. This is because the type(uintX).max function has to dynamically calculate the maximum value of a uint256, which can be expensive in terms of gas. Constants, on the other hand, are stored in the bytecode of your contract, so they do not have to be recalculated every time you need them. Saves 13 GAS

FILE: 2023-07-axelar/contracts/its/interchain-token/InterchainToken.sol

56:  if (allowance_ != type(uint256).max) {

57:  if (allowance_ > type(uint256).max - amount) {

58:  allowance_ = type(uint256).max - amount;

88: if (_allowance != type(uint256).max) {

95: if (allowance_ != type(uint256).max) {

96: if (allowance_ > type(uint256).max - amount) {
                    
97: allowance_ = type(uint256).max - amount;

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token/InterchainToken.sol#L56-L58

[G-11] Splitting require()/if() statements that use && saves gas

Saves 52 GAS,4 Instance

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper by 13 gas

FILE: 2023-07-axelar/contracts/its/remote-address-validator/RemoteAddressValidator.sol

58:  if ((b >= 65) && (b <= 70)) bytes(s)[i] = bytes1(b + uint8(32));

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L58

FILE: 2023-07-axelar/contracts/cgp/AxelarGateway.sol

88: if (msg.sender != getAddress(KEY_MINT_LIMITER) && msg.sender != getAddress(KEY_GOVERNANCE)) revert NotMintLimiter();

446: if (!success || (returnData.length != uint256(0) && !abi.decode(returnData, (bool)))) revert BurnFailed(symbol);

635: if (limit > 0 && amount > limit) revert ExceedMintLimit(symbol);

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/AxelarGateway.sol#L88

[G-12] Caching global variables is more expensive than using the actual variable (use msg.sender instead of caching it)

The msg.sender variable is a special variable that always refers to the address of the sender of the current transaction. This variable is not stored in memory, so it is much cheaper to use than a cached global variable.

FILE: Breadcrumbs2023-07-axelar/contracts/its/interchain-token-service/InterchainTokenService.sol

- 348:  address deployer_ = msg.sender;
- 349:        tokenId = getCustomTokenId(deployer_, salt);
+ 349:        tokenId = getCustomTokenId(msg.sender, salt);
350:        _deployTokenManager(tokenId, tokenManagerType, params);
- 351:        emit CustomTokenIdClaimed(tokenId, deployer_, salt);
+ 351:        emit CustomTokenIdClaimed(tokenId, msg.sender, salt);


-        address deployer_ = msg.sender;
-         tokenId = getCustomTokenId(deployer_, salt);
+         tokenId = getCustomTokenId(msg.sender, salt);
        _deployRemoteTokenManager(tokenId, destinationChain, gasValue, tokenManagerType, params);
-        emit CustomTokenIdClaimed(tokenId, deployer_, salt);
+        emit CustomTokenIdClaimed(tokenId, msg.sender, salt);


-  address caller = msg.sender;
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
        IERC20 token = IERC20(tokenManager.tokenAddress());

-        SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);
+        SafeTokenTransferFrom.safeTransferFrom(token, msg.sender, destinationAddress, amount);
-        _setExpressReceiveToken(tokenId, destinationAddress, amount, commandId, caller);
+   _setExpressReceiveToken(tokenId, destinationAddress, amount, commandId, msg.sender);


- address caller = msg.sender;
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
        IERC20 token = IERC20(tokenManager.tokenAddress());

-        SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);
+        SafeTokenTransferFrom.safeTransferFrom(token, msg.sender, destinationAddress, amount);
        _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);

-        _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
+        _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, msg.sender);

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L372-L375

FILE: Breadcrumbs2023-07-axelar/contracts/its/interchain-token/InterchainToken.sol

- address sender = msg.sender;
        ITokenManager tokenManager = getTokenManager();
        /**
         * @dev if you know the value of `tokenManagerRequiresApproval()` you can just skip the if statement and just do nothing or _approve.
         */
        if (tokenManagerRequiresApproval()) {
-            uint256 allowance_ = allowance[sender][address(tokenManager)];-            
+           uint256 allowance_ = allowance[msg.sender][address(tokenManager)];
            if (allowance_ != type(uint256).max) {
                if (allowance_ > type(uint256).max - amount) {
                    allowance_ = type(uint256).max - amount;
                }

-                _approve(sender, address(tokenManager), allowance_ + amount);
+                _approve(msg.sender, address(tokenManager), allowance_ + amount);
            }

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token/InterchainToken.sol#L49

#0 - c4-pre-sort

2023-07-29T01:15:02Z

0xSorryNotSorry marked the issue as high quality report

#1 - c4-judge

2023-09-04T19:34:18Z

berndartmueller marked the issue as grade-a

#2 - c4-judge

2023-09-04T19:42:14Z

berndartmueller marked the issue as selected for report

Findings Information

🌟 Selected for report: pcarranzav

Also found by: Jeiwan, K42, MrPotatoMagic, Sathish9098, libratus, niloy

Labels

analysis-advanced
grade-a
A-03

Awards

827.4542 USDC - $827.45

External Links

Analysis - Axelar Network

Summary

ListHeadDetails
1Overview of Axelar Network Protocoloverview of the key components and features of Axelarb Network
2My ThoughtsMy own thoughts about future of the this protocol
3Audit approachProcess and steps i followed
4LearningsLearnings from this protocol
5Possible Systemic RisksThe possible systemic risks based on my analysis
6Code CommentarySuggestions for existing code base
7Centralization risksConcerns associated with centralized systems
8Gas OptimizationsDetails about my gas optimizations findings and gas savings
9RisksPossible risks
10Non-functional aspectsgeneral suggestions
11Time spent on analysisThe Over all time spend for this reports

Overview

Axelar is a decentralized interoperability network connecting all blockchains, assets and apps through a universal set of protocols and APIs.

  • It is built on top off the Cosmos SDK
  • Use Axelar network to send tokens between any Cosmos and EVM chains.They can also send arbitrary messages between chains
  • The relayer (can be anyone due to being permissionless) triggers the execute method automatically once the message has been confirmed by the Axelar network.

My Thoughts

Axelar could change the future in following areas

  • Decentralized finance (DeFi): Axelar could make it easier for users to access DeFi applications on different chains. This could help to increase the liquidity of DeFi markets and could also make DeFi applications more accessible to users.

  • NFTs: Axelar could make it easier for users to trade NFTs between different chains. This could help to increase the liquidity of NFT markets and could also make NFTs more accessible to users.

  • Gaming: Axelar could make it easier for users to play games across different chains. This could help to increase the reach of blockchain games and could also make games more immersive for users.

Audit approach

I followed below steps while analyzing and auditing the code base.

  1. Read the contest Readme.md and took the required notes.
  • Axelar Network protocol uses
    • Uses Inheritance
    • Follows the ERC20 Standards
    • Uses TimeLocks
    • Multi-chain
  1. Analyzed the over all codebase one iterations very fast

  2. Study of documentation to understand each contract purpose, its functionality, how it is connected with other contracts, etc.

  3. Then i read old audits and already known findings. Then go through the bot races findings

  4. Then setup my testing environment things. Run the tests to checks all test passed.

  5. Finally, I started with the auditing the code base in depth way I started understanding line by line code and took the necessary notes to ask some questions to sponsors.

Stages of audit

  • The first stage of the audit

During the initial stage of the audit for Axelar Network, the primary focus was on analyzing gas usage and quality assurance (QA) aspects. This phase of the audit aimed to ensure the efficiency of gas consumption and verify the robustness of the platform.

  • The second stage of the audit

In the second stage of the audit for Axelar Network, the focus shifted towards understanding the protocol usage in more detail. This involved identifying and analyzing the important contracts and functions within the system. By examining these key components, the audit aimed to gain a comprehensive understanding of the protocol's functionality and potential risks.

  • The third stage of the audit

During the third stage of the audit for Axelar Network, the focus was on thoroughly examining and marking any doubtful or vulnerable areas within the protocol. This stage involved conducting comprehensive vulnerability assessments and identifying potential weaknesses in the system. Found 60-70 vulnerable and weakness code parts all marked with @Audit tags.

  • The fourth stage of the audit

During the fourth stage of the audit for Axelar Network, a comprehensive analysis and testing of the previously identified doubtful and vulnerable areas were conducted. This stage involved diving deeper into these areas, performing in-depth examinations, and subjecting them to rigorous testing, including fuzzing with various inputs. Finally concluded findings after all research's and tests. Then i reported C4 with proper formats

Learnings

  • Axelar's architecture: The Axelar network is composed of three main components: the relayers, the validators, and the gateway smart contracts. The relayers are responsible for transferring messages between chains, the validators are responsible for confirming events and commands on different chains, and the gateway smart contracts are responsible for managing the transfer of tokens and data between chains.

  • Axelar's security model: Axelar's security model is based on a combination of decentralized validators and secure smart contracts. The decentralized validators are responsible for confirming events and commands on different chains, and the secure smart contracts are responsible for managing the transfer of tokens and data between chains.

  • Axelar's roadmap: The Axelar team has a roadmap for the development of the network. The roadmap includes plans to add support for more chains, to improve the security of the network, and to make the network more user-friendly.

Possible Systemic Risks

  • Relayer attacks: A relayer could potentially attack the Axelar network by submitting malicious commands to the gateway smart contracts. This could result in the theft of tokens or the loss of data.

  • Double-spend attacks: A user could potentially double-spend tokens by sending them to two different chains. This could be done by sending the tokens to one chain and then submitting a malicious command to the gateway smart contracts to cancel the transaction on the other chain.

  • Sybil attack: A malicious actor could potentially create a large number of fake validators in order to gain control of the Axelar network. This could allow the malicious actor to approve malicious commands or to prevent legitimate commands from being processed.

  • Buggy smart contracts: The Axelar gateway smart contracts could contain bugs that could be exploited by attackers. This could result in the theft of tokens or the loss of data.

Code Commentary

  • Events is missing indexed fields Index event fields make the field more quickly accessible to off-chain.Each event should use three indexed fields if there are three or more fields.

  • Shadowing variables can be confusing in contracts, as it can be difficult to track which variable is being referenced at any given time. This can lead to errors and unexpected behavior.

    • operator shadowed varibale used in InterchainTokenService.sol. The same varibale name used in Operator.sol
    • gateway shadowed varibale used in InterchainGovernance.sol. The same varibale name used in AxelarExecutable .sol as IAxelarGateway instance
    • gateway, governanceChain, governanceAddress used in AxelarServiceGovernance.sol
  • Use latest version of solidity. Use atleast 0.8.17

  • Don't use bool . Use uint(1) and uint(2) for true,false

  • Use abi.encode instead of abi.encodePacked when ever possible to avoid hash collisions

  • Use notPaused Modifier in important functions

  • Consider using a struct to represent trusted addresses instead of separate arrays for chain names and addresses. This can enhance code readability and make it easier to manage trusted addresses

  • Explicitly specify the visibility modifiers for functions and state variables, such as public, external, internal, or private, based on their intended use

  • Consider using bytes32 for governanceChain and governanceAddress if they are fixed-size strings to save gas costs

  • Add informative error messages in require statements to provide clear feedback on revert reasons

  • Add safeguards to reentrnacy attacks

  • Use enum values directly instead of converting commandId to an IAxelarGateway.Command enum in the execute and executeWithToken functions, use the enum values directly to simplify the code.

  • Add validations in MultisigBase.sol the constructor to ensure that the provided list of signers is not empty and that the threshold is not zero

  • Move the voting logic out of the onlySigners modifier and into a separate function, which can be called explicitly when a signer wants to vote on a specific topic. This separation improves code readability and allows for better control over when a vote is cast

  • Some important state changes might not be adequately captured by events. Consider emitting events for significant state changes to improve transparency and allow external systems to track contract activity

  • Add detailed comments explaining the purpose and functionality of each function. This can significantly improve code readability and ease maintenance

  • Ensure consistent naming conventions throughout the codebase to enhance clarity and reduce confusion

  • Consider breaking the contract into smaller, modular components. This can make the codebase more manageable and easier to maintain

Centralization risks

A single point of failure is not acceptable for this project Centrality risk is high in the project, the role of onlyOwnerdetailed below has very critical and important powers

Project and funds may be compromised by a malicious or stolen private key onlyOwner msg.sender

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L96-L96

92:     function setWhitelistedProposalCaller(
93:         string calldata sourceChain,
94:         address sourceCaller,
95:         bool whitelisted
96:     ) external override onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L111-L111

107:     function setWhitelistedProposalSender(
108:         string calldata sourceChain,
109:         address sourceSender,
110:         bool whitelisted
111:     ) external override onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/gmp-sdk/upgradable/Upgradable.sol#L56-L56

52:     function upgrade(
53:         address newImplementation,
54:         bytes32 newImplementationCodeHash,
55:         bytes calldata params
56:     ) external override onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L547-L547


547:     function setPaused(bool paused) external onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L83-L83

83:     function addTrustedAddress(string memory chain, string memory addr) public onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L95-L95


95:     function removeTrustedAddress(string calldata chain) external onlyOwner

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L106-L106


106:     function addGatewaySupportedChains(string[] calldata chainNames) external onlyOwner  

https://github.com/code-423n4/2023-07-axelar/tree/main/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L119-L119


119:     function removeGatewaySupportedChains(string[] calldata chainNames) external onlyOwner 

Gas Optimizations

In the context of optimizing gas consumption, several improvements have been identified within the codebase of the Axelar protocol. One of the main optimizations involves combining multiple address mappings into a single mapping of an address to a struct where appropriate. By packing the structs and modifying the uint type for the values, storage reads become more cost-efficient, resulting in substantial gas savings. Additionally, using the calldata keyword instead of memory for read-only arguments in external functions reduces gas usage significantly, as it avoids unnecessary data copying.

Another gas optimization relates to avoiding the overhead of using bool storage, which incurs extra gas costs due to read and write operations. By replacing bool variables with appropriate uint values (e.g., 1 and 2) to represent true and false, the contract can avoid additional SLOAD operations, leading to substantial gas savings across multiple instances.

Furthermore, using low-level calls instead of external function calls can also save gas by eliminating contract existence checks. This is particularly useful for functions that are expected to revert when called by regular users since it avoids unnecessary opcode execution.

To further reduce gas consumption, the codebase can be updated to use calldata pointers instead of memory pointers, which offer cost savings in loop iterations. Additionally, moving IF and require() statements that check input arguments to the top of functions can avoid unnecessary computation and potential gas waste.

Marking functions guaranteed to revert when called by normal users as payable is another optimization to reduce gas costs, as it avoids the extra gas spent on checking whether a payment was provided.

To enhance the readability and efficiency of the code, optimizing names for functions and variables can lead to gas savings by reducing method IDs and method call costs.

Finally, by eliminating unnecessary default value initialization, such as explicitly setting variables to their default values, gas savings can be achieved by allowing variables to use their default values automatically.

Overall, implementing these gas optimizations can significantly enhance the efficiency and cost-effectiveness of the Axelar protocol, making it more robust and scalable for its users

Risks

  • The contract uses strings for governanceChain and governanceAddress. While it's possible to use strings for these purposes, it's generally better to use bytes32 for fixed-size strings to save gas costs and avoid potential security vulnerabilities. If the governance chain and address are known to have fixed lengths, consider using bytes32 instead.

  • contract emits events such as ProposalExecuted, ProposalScheduled, and ProposalCancelled, there are no comments or documentation explaining what these events signify and the expected format of the emitted data.

  • Lack of access control in executeProposal() function

  • The target address could be an invalid or malicious contract address, leading to unexpected behavior or potential vulnerabilities. Ensure that you add appropriate input validation to check the validity of the target address and other inputs.

  • The rotateSigners function is used to both rotate the signers and set the initial signers. It might be confusing to use the same function for both purposes. Consider splitting the functionality into two separate functions, one for the initial setup and another for rotating the signers

  • The getSignerVotesCount function iterates through all signers to count the votes for a topic. This operation has a linear complexity (O(n)), where n is the number of signers.

  • The onlySigners modifier modifies contract state by updating the vote count and setting the hasVoted mapping. There could be potential reentrancy vulnerabilities if any function called within the modifier allows external contract calls before the state modifications are completed

  • The contract uses keccak256(msg.data) to generate the vote topic hash. Using msg.data directly can lead to inconsistent hash generation due to differences in ABI encoding

  • The contract only provides a single signer threshold for executing operations. In more complex multisignature contracts, it might be beneficial to have different levels of signers with different threshold requirements for certain types of transactions

  • The contract does not perform any signature verification to ensure that the transactions are signed correctly by the required number of signers. Without proper signature validation, the security of the multisignature contract may be compromised

  • Some functions are marked as onlySelf, which means they can only be called by the contract itself. Make sure that these functions are intended for internal use only and cannot be called by unauthorized parties

  • Limit the use of delegatecall to only trusted and thoroughly audited contracts. If possible, avoid using delegatecall altogether as it can introduce complex security risks

  • return codehash != bytes32(0) && codehash != 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 avoid the hardcoded value check. This will cause problems in in future.

  • Be cautious with loops and array processing. If the number of interchainCalls in the sendProposals function is significant, it could cause the transaction to exceed the block gas limit, leading to a revert. Ensure that the gas limit is well within the block limit.

  • InterchainProposalSender.sol contract doesn't have any nonce handling mechanism to prevent replay attacks. Consider adding a nonce or some other form of unique identifier to each proposal to prevent replay attacks

  • The contract inherits from AxelarExecutable, which likely implements a fallback function. Make sure to review the fallback function's implementation in the AxelarExecutable contract to ensure it behaves as expected

  • The contract ConstAddressDeployer.sol uses create2 to deploy contracts. While this is generally safe and useful for deterministic contract deployments, be aware that contract deployment might consume more gas compared to standard create calls. Additionally, the size of the deployment bytecode should be kept in check to avoid potential out-of-gas issues

  • The contract uses assembly blocks to read and write from/to storage slots. While this can be an efficient way to handle storage operations, it increases the complexity of the code. Consider carefully evaluating the need for assembly and use it only when necessary to optimize gas costs

  • The _popExpressReceiveToken and _popExpressReceiveTokenWithData functions return the address expressCaller, but it is not used within the functions. If the return value is not needed, you can remove the assignment and just return early when the express caller is not found

  • When accessing storage slots with sload, be cautious about reading from slots that have not been written before. For example, you can initialize storage slots to a default value (e.g., 0) before using them

  • The _getExpressReceiveTokenSlot and _getExpressReceiveTokenWithDataSlot functions use keccak256 to calculate slots based on various parameters. Ensure that the combination of parameters used to calculate slots results in unique slots for each express call to avoid potential storage slot collisions

  • The contract inherits from Upgradable, which might imply that this contract is designed to be upgradable. Upgradable contracts can be complex and require careful consideration to maintain security and compatibility across versions

Non-functional aspects

  • Aim for high test coverage to validate the contract's behavior and catch potential bugs or vulnerabilities
  • The protocol execution flow is not explained in efficient way.
  • Its best to explain over all protocol with architecture is easy to understandings
  • Consider designing the contract to be upgradable or allow for versioning. This can help address issues, introduce new features, or adapt to evolving requirements without disrupting the entire system
  • Consider using standardized security libraries like OpenZeppelin to minimize vulnerabilities

Time spent on analysis

15 Hours

Time spent:

14 hours

#0 - c4-judge

2023-09-04T10:55:12Z

berndartmueller marked the issue as grade-b

#1 - sathishpic22

2023-09-08T19:12:46Z

Hi @berndartmueller

I received a lower grade than I expected, and I'm not sure what went wrong in my reports.

I wrote more detailed reports and followed the C4 format as instructed.

My report contains

  • Overview
  • Code commentary
  • Gas optimizations
  • Possible risks based on codebase

I reviewed all the Grade A reports, and I believe my report is superior https://github.com/code-423n4/2023-07-axelar-findings/issues/439 , https://github.com/code-423n4/2023-07-axelar-findings/issues/376 , https://github.com/code-423n4/2023-07-axelar-findings/issues/206 to some of them.

Could you kindly reevaluate my report? If there are any areas that require improvement, I would greatly appreciate your feedback to enhance my analysis reports for future submissions.

Thank you

#2 - berndartmueller

2023-09-09T19:51:48Z

Hey @sathishpic22!

I'd like to point out that your submitted analysis report is an "advanced" report, and you compared it to "basic" analysis reports:

I reviewed all the Grade A reports, and I believe my report is superior https://github.com/code-423n4/2023-07-axelar-findings/issues/439 , https://github.com/code-423n4/2023-07-axelar-findings/issues/376 , https://github.com/code-423n4/2023-07-axelar-findings/issues/206 to some of them.

Nevertheless, after reviewing your submission again, I've decided to change the grade to "A". Please note that I see your report somewhere between grade "B" and "A". Mainly because your report seems more like a QA and Gas report smashed together. For an advanced analysis report to be graded A (or even selected as the primary report), it's important to analyze the codebase from a more general and high-level view and point out issues that are more high-level (see https://docs.code4rena.com/awarding/judging-criteria#analysis)

#3 - c4-judge

2023-09-09T19:52:03Z

berndartmueller marked the issue as grade-a

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