code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

Avoid `payable(address).transfer` #186

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L22-L23 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L51-L52 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L71-L72 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L84-L86 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L128-L129 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L144-L145

Vulnerability details

The Axelar contracts use <address payable>.transfer to perform native token transfers:

ReceiverImplementation#receiveAndSendToken:

        // Always refunding native otherwise it's sent on DepositReceiver self destruction
        if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

ReceiverImplementation#receiveAndSendNative:

            if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

ReceiverImplementation#receiveAndUnwrapNative L#71:

        if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

ReceiverImplementation#receiveAndUnwrapNative L#84:

        // Unwrapping the token into native currency and sending it to the recipient
        IWETH9(wrappedTokenAddress).withdraw(amount);
        recipient.transfer(amount);

AxelarGasService#collectFees:

                if (amount > 0) receiver.transfer(amount);

AxelarGasService#refund:

            receiver.transfer(amount);

However, transfer forwards a fixed stipend of 2300 gas that may be insufficient for some smart contract recipients, and could potentially revert in the future if gas costs change. (See the Consensys Diligence article here).

Impact: Some refund recipients and receivers, especially custom contracts or smart contract wallets, may be unable to receive native token transfers, breaking composability of the Axelar protocol.

Suggestion: Use <address payable>.call to perform native token transfers. However, note that forwarding unlimited gas introduces a potential vector for re-entrancy.

GalloDaSballo commented 2 years ago

See #203

re1ro commented 2 years ago

Duplicate of #4