We shouldn't have to write risky code to use the Interchain Token Service

It appears that Axelar staff are not taking the security issue that I had highlighted last month:

github com/axelarnetwork/interchain-token-service/issues/101

(This forum platform doesn’t let me post links, I get the error: “Sorry you cannot post a link to that host.”)

The issue is big enough that Coinbase has published at least 4 articles that cover this kind of issue (“superuser risk”) within the past year:

How Coinbase Protects Users From Risky Assets
coinbase com/blog/how-coinbase-protects-users-from-risky-assets

How Coinbase reviews tokens on Ethereum for technical security risks
coinbase com/blog/how-coinbase-reviews-tokens-on-ethereum-for-technical-security-risks

Security PSA: Protecting ERC-20 assets from malicious actors
coinbase com/blog/security-psa-protecting-erc-20-assets-from-malicious-actors

Token Custody Risks: Defining Security in the Crypto World
coinbase com/blog/token-custody-risks-defining-security-in-the-crypto-world

In the third one I’ve listed above, they even explicitly show this code:

… which looks almost exactly like the sample code that Axelar has published:

    function burn(address from, uint256 amount) external onlyOwner {
        _burn(from, amount);

Source: github com/axelarnetwork/axelar-docs/blob/5c2cb34c424a0c28f2dafff24f04c08a0ca4f6e8/public/samples/interchain-token-iinterchaintoken.sol#L41-L43

I’m aware that in the case of the Interchain Token Service, the suggestion is to transfer ownership of the contract to the token manager so that it can freely burn and mint the tokens. But that would mean token issuers would completely lose control of the contract, so they’d be against such a crazy suggestion. So in a separate GitHub issue about that I suggested replacing onlyOwner with onlyRole (function burn(address from, uint256 amount) external onlyRole(BURNER_ROLE)) , assigning the token manager burner role and minter role. But that is still insecure because an admin of the contract could one day assign the burner role to herself and burn everyone’s tokens.

The proper way to do it is instead of having token issuers add such a risky burn function, request approval from the token holder (by calling permit or approve) and call burnFrom or transferFrom when executing an interchain transfer. That’s how Chainlink’s Cross-Chain Interoperability Protocol (CCIP) does it, as I’ve explained with code references in the GitHub issue.

The risk for Axelar is the possibility that one or more tokens that contains your suggested burn function gets compromised by an admin of the token, causing loss of others’ tokens. The blame would go back to Axelar for condoning such risky code and dismissing the concerns that are being raised in this present time, causing reputational damage, loss of AXL token value, etc… I really hope that would never happen.

Luckily the Interchain Token Service is not yet live on mainnets, so @sergey please correct this and any other critical issues while you can do so more easily than if it was in production.

1 Like

This report is cross-posted from the original issue on github:

The report was qualified as invalid due to several reasons:

  • Mentioned code is not a part of the Interchain Token Service itself but rather a simplified example code from the documentation about how to create custom tokens.
  • Any of the “superuser” risks described above are not applicable as onlyOwner role in this example is granted to the ITS TokenManager contact and not any EVM user account.
  • There is nothing stopping developers from implementing approve + burnFrom functionality in their custom interchain tokens if they choose so.

For complete Interchain Token implementation you can always use StandardizedToken as a reference.

If onlyOwner is replaced with onlyRole(BURNER_ROLE) (if the token issuer doesn’‘t want to give up ownership) then whoever manages roles in the token contract (e.g. token contract admin) is the “superuser”, and could grant herself BURNER_ROLE, then go onto burn others’ tokens. That is definitely a risk, so the warnings that both I and Coinbase have made are valid in this case.

It’s the token manager that does the burning. Currently it calls the risky function burn. Which means we are forced to include that risky function in order to be able to use ITS. I’m saying that there needs to be a way to do it without the risky function, e.g. have token manager call burnFrom or transferFrom (after a permit or approve). Token developers can imoprt ERC20Burnable to get burnFrom but it’s up to Axelar to make the token manager use it.

Developers will copy example code that Axelar has published, so it should be good quality and without risky code.

The purpose of Sample Token example is to showcase integration point with the token manager. Not give developers a copy-paste solution. In our example onlyOwner is only for mint + burn role granted to the Token Manager. There is no ownership. It’s not a contract admin and there is no account holding this role. Token Manager can’t transfer that permission to anyone else.

If we include the whole Open Zeppeling framework in our example code it will make understanding base ITS concepts even harder. It is developer responsibility to know what are they doing or just use a Standard Interchain Token provided by the service. Giving a different example for a copy-paste will not prevent unexperienced developers from introducing security problems.

From our github conversation the problem is with your custom token implementation where you also have ADMIN_ROLE that can do too many dangerous things. This is the “superuser” risk that you are introducing in your token. Our simple example doesn’t have that “risky code” in it.

approve + burnFrom in our example will not improve the security model anyhow, but will make users pay extra gas for every cross-chain transaction. If you have self-sabotaging admins in your token, then yeah you can make your burn function to work like burnFrom and require approve. I bet it won’t help with anything as admin can still destroy your token in many ways without any burn function.

Please stop posting the same self-contradicting statements, they do not make sense at this point. If you still have concerns or need help with implementing your custom token please schedule a call with me using the provided link.