Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas fees are refunded to a wrong address when transferring tokens via InterchainToken.interchainTransferFrom #316

Open
code423n4 opened this issue Jul 21, 2023 · 7 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) downgraded by judge Judge downgraded the risk level of this issue M-05 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

In a case when gas fees are refunded for a token transfer made via the InterchainToken.interchainTransferFrom function, the fees will be refunded to the owner of the tokens, not the address that actually paid the fees. As a result, the sender will lose the fees paid for the cross-chain transaction and will not receive tokens on the other chain; the owner of the token will have their tokens and will receive the fees.

Proof of Concept

The InterchainToken.interchainTransferFrom function is used to transfer tokens cross-chain. The function is identical to the ERC20.transferFrom function: an approved address can send someone else's tokens to another chain. Since this is a cross-chain transaction, the sender also has to pay the additional gas fee for executing the transaction:

  1. the function calls tokenManager.transmitInterchainTransfer;
  2. tokenManager.transmitInterchainTransfer calls interchainTokenService.transmitSendToken;
  3. interchainTokenService.transmitSendToken calls _callContract;
  4. _callContract uses the msg.value to pay the extra gas fees.

Notice that the gasService.payNativeGasForContractCall call in _callContract takes the refundTo address, i.e. the address that will receive refunded gas fee. If we return up on the call stack, we'll see that the refund address is the sender address that's passed to the tokenManager.transmitInterchainTransfer call. Thus, gas fees will be refunded to the token owner, not the caller, however it's the caller who pays them.

Tools Used

Manual review

Recommended Mitigation Steps

The TokenManager.transmitInterchainTransfer and the InterchainTokenService.transmitSendToken functions, besides taking the sender/sourceAddress argument, need also take the "refund to" address. In the InterchainToken.interchainTransferFrom function, the two argument will be set to different addresses: the sender/sourceAddress argument will be set to the token owner address; the new "refund to" argument will be set to msg.sender. Thus, while tokens will be taken from their owner, the cross-chain gas fees will be refunded to the actual transaction sender.

Assessed type

ETH-Transfer

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jul 21, 2023
code423n4 added a commit that referenced this issue Jul 21, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as primary issue

@c4-sponsor
Copy link

deanamiel marked the issue as disagree with severity

@c4-sponsor c4-sponsor added the disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) label Aug 25, 2023
@deanamiel
Copy link

Corrected Severity: QA
In most cases it will have been called by the sender anyway, so having the sender be refunded is the desired effect. Sometimes this will not be the case though, depending on the use case. Therefore, we have added a parameter to keep track of where the funds need to be refunded.
Link to public PR:
axelarnetwork/interchain-token-service#96

@berndartmueller
Copy link
Member

Considering this medium severity as per "...In most cases it will have been called by the sender anyway" (sponsors statement above). Nevertheless, in the cases where an approved address transfers the tokens, the gas is incorrectly refunded.

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Sep 1, 2023
@c4-judge
Copy link

c4-judge commented Sep 1, 2023

berndartmueller changed the severity to 2 (Med Risk)

@c4-judge c4-judge added the downgraded by judge Judge downgraded the risk level of this issue label Sep 1, 2023
@c4-judge
Copy link

c4-judge commented Sep 1, 2023

berndartmueller marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Sep 1, 2023
@C4-Staff C4-Staff added the M-05 label Sep 13, 2023
@milapsheth
Copy link

We acknowledge the severity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) downgraded by judge Judge downgraded the risk level of this issue M-05 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report
Projects
None yet
Development

No branches or pull requests

8 participants