QA Report

[code423n4]

Transferring dust native fund is not related to addWrapping

https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L63

    function addWrapping(
        string calldata symbol,
        address xc20Token,
        string memory newName,
        string memory newSymbol
    ) external payable onlyOwner {
        address axelarToken = gateway().tokenAddresses(symbol);
        if (axelarToken == address(0)) revert('NotAxelarToken()');
        if (xc20Token.codehash != xc20Codehash) revert('NotXc20Token()');
        if (wrapped[axelarToken] != address(0)) revert('AlreadyWrappingAxelarToken()');
        if (unwrapped[xc20Token] != address(0)) revert('AlreadyWrappingXC20Token()');
        wrapped[axelarToken] = xc20Token;
        unwrapped[xc20Token] = axelarToken;
        if (!LocalAsset(xc20Token).set_team(address(this), address(this), address(this))) revert('NotOwner()');
        if (!LocalAsset(xc20Token).set_metadata(newName, newSymbol, IERC20(axelarToken).decimals())) revert('CannotSetMetadata()');
        payable(msg.sender).transfer(address(this).balance);
    }

Why transfer dust fund using addWrapper? Consider adding a separate function to do this.

Use payable(...).call{value: ...}("") instead of .transfer

Using deprecated transfer() on address payable may revert in these cases:

1. The withdraw recipient is a smart contract that does not implement a payable function.
2. The withdraw recipient is a smart contract that does implement a payable fallback which uses more than 2300 gas unit.
3. The withdraw recipient is a smart contract that implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.

Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

An example on Axelar codebase that using .transfer is in https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L144

receiver.transfer(amount);

Should be

      (bool success, ) = payable(receiver).call{value: amount}("");
      require(success, "ETH transfer failed");

XC20Wrapper is not emitting any event

XC20Wrapper should emit events on wrap / unwrap / receive cross chain fund (_executeWithToken) to allow indexers to track the transactions.

_isSortedAscAndContainsNoDuplicate failed if accounts.length == 0

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

It is using accounts.length - 1 which will be underflow and revert when accounts.length == 0

Consider adding if to skip this case

    function _isSortedAscAndContainsNoDuplicate(address[] memory accounts) internal pure returns (bool) {
        if (accounts.length == 0) return false;

        for (uint256 i; i < accounts.length - 1; ++i) {
            if (accounts[i] >= accounts[i + 1]) {
                return false;
            }
        }

        return accounts[0] != address(0);
    }

Missing minimum newThreshold check

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

if (newThreshold == 0 || totalWeight < newThreshold) revert InvalidThreshold();

New threshold shouldn't be too low such that just one low weight operator can execute malicious transaction.

It would be great to have some more shield on this important function by checking new threshold to be more than xx% of totalWeight

Should use pragma solidity ^0.8.9; instead of pragma solidity 0.8.9;

pragma solidity ^0.8.9; allow application that import this library to use more recent version of solidity 0.8.x such as 0.8.13 to compile the contracts while pragma solidity 0.8.9; doesn't allow it.

If that application use some library that require ^0.8.13. That application may not be able to used with Axelar.

AxelarDepositService is not support general message passing

We cannot see anything related to contract calling such as target contract address or payload in the https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol.

Which means it doesn't support general message passing

XC20 token doesn't support general message passing on auto xc20 wrap

    function _executeWithToken(
        string calldata,
        string calldata,
        bytes calldata payload,
        string calldata tokenSymbol,
        uint256 amount
    ) internal override {
        address receiver = abi.decode(payload, (address));
        address tokenAddress = gateway().tokenAddresses(tokenSymbol);
        address xc20 = wrapped[tokenAddress];
        if (xc20 == address(0) || !LocalAsset(xc20).mint(receiver, amount)) {
            _safeTransfer(tokenAddress, receiver, amount);
        }
    }

payload only hold receiver which is not sufficient for general message passing (require target contract and payload). Moreover, there aren't any logic involve contract calling on _executeWithToken.

[GalloDaSballo]

Transferring dust native fund is not related to addWrapping

Valid R

Use payable(...).call{value: ...}("") instead of .transfer

L

XC20Wrapper is not emitting any event

NC

 _isSortedAscAndContainsNoDuplicate failed if accounts.length == 0

Don't think you need this, NC

Missing minimum newThreshold check

Valid L

Should use pragma solidity ^0.8.9; instead of pragma solidity 0.8.9;

Disagree, your colleagues would be sending "floating pragma" reports.

AxelarDepositService is not support general message passing

Disputing in lack of detail, please add more information next time (what would the fixed system look like?)

XC20 token doesn't support general message passing on auto xc20 wrap

Also not convinced by this as the codebase doesn't seem to use it

Overall a genuine report, thank you!

2L 1R 2NC