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

0 stars 0 forks source link

`Transfer()` is used instead of `Call()` to transfer ETH #166

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L23 https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L51 https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L23 https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L51 https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/gas-service/AxelarGasService.sol#L128 https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/gas-service/AxelarGasService.sol#L144 https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/xc20/contracts/XC20Wrapper.sol#L63

Vulnerability details

The use of .transfer() or .send() to send ether is now considered bad practice as gas costs can change, which would break the code.

The transaction will fail inevitably when:

Affected Codes

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L23

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L51

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L23

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/deposit-service/ReceiverImplementation.sol#L51

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/gas-service/AxelarGasService.sol#L128

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/gas-service/AxelarGasService.sol#L144

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/xc20/contracts/XC20Wrapper.sol#L63

Recommendation

Use call instead of transfer to send ether. And return value must be checked if sending ether is successful or not. make sure to check for reentrancy.

GalloDaSballo commented 2 years ago

See #203

re1ro commented 2 years ago

Duplicate of #4