XC20Wrapper: Unsupported fee-on-transfer tokens

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L75-L88

Vulnerability details

Impact

In the XC20Wrapper contract, if axelarToken is TokenType.External and is a fee-on-transfer token, the amount of tokens received by the contract in the wrap function will be less than amount, but the amount of wrappedToken minted for the user is amount.
The user can drain the axelarToken in the contract by constantly calling the wrap and unwrap functions

Proof of Concept

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L75-L88

Tools Used

None

Recommended Mitigation Steps

Consider getting the received amount by calculating the difference of token balance (using balanceOf) before and after the transferFrom.

[Foivos]

xc20Wrapper in only meant to be used with non-External tokens. External tokens already exist on moonbeam and they should figure out their own way to go to the rest of Polkadot.

[re1ro]

Duplicate of #16

[GalloDaSballo]

The warden has shown that the contract for wrapping will not check for the change in balance for the token after the transfer, this will cause issues with both rebasing and feeOnTransfer tokens.

The system requires the owner to addWrapping meaning that the issue could happen only if the token is feeOnTransfer and the Owner decided to allow that token to be wrappable.

The total loss will amount to the cost of the fees, specifically the last few to call unwrap will not be able to receive any token (as all tokens will have already been claimed)

Given that the max loss is some balances, but it is contingent on the admin allowing those tokens, and the token itself being feeOnTransfer, I think Low Severity to be more appropriate.

I recommend end users to check if any token will have fees and recommend the sponsor to either refactor or simply never enable such a token