code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L38 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L64 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L30

Vulnerability details

Not calling approve(0) before setting a new approval causes the call to revert when used with Tether (USDT)

Impact

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value (it will revert if the current approval is not zero to protect against front-running changes of approvals). These tokens must first be approved for zero and then the actual allowance can be approved.

Proof of Concept

File: ReceiverImplementation.sol line 38

        IERC20(tokenAddress).approve(gateway, amount);

File: ReceiverImplementation.sol line 64

        IERC20(wrappedTokenAddress).approve(gateway, amount);

File: AxelarDepositService.sol line 30

        IERC20(wrappedTokenAddress).approve(gateway, amount);

Tools used

Manual code review

Recommended Mitigation Steps

Use approve(gateway, 0) to set the allowance to zero immediately before each of the existing approve() calls.

GalloDaSballo commented 2 years ago

Line below will set approval to 0 as it transfer all: https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L39