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

0 stars 0 forks source link

Use safetransfer/safetransferFrom consistently instead of transfer/transferFrom #210

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/gas-service/AxelarGasService.sol#L128 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L144 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L23 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L86

Vulnerability details

Impact

Its a good to add require() statement to checks the return value of token transfer or using safetransfer or safetransferFrom on Openzeppelin to ensure the token revert when transfer failure. Failure to do so will cause silent failures of transfer and affect token accountng in contract.

Proof of Concept

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

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

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

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

Tools Used

Manual review code

Recommended Mitigation Steps

consider using safetransfer/safetransferFrom or require() consistently

GalloDaSballo commented 2 years ago

safeTransfer is used: https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L131