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

0 stars 0 forks source link

Some functions calls native payable.transfer, which can be unusable for smart contract calls #147

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#L23-L24

Vulnerability details

Impact

The refundAddress is unchanged in the DepositReceiver contract with a fixed address. In the ReceiverImplementation contract, receiveAndSendToken/receiveAndSendNative/receiveAndUnwrapNative functions call native payable.transfer on the refundAddress. This is unsafe as transfer has hard coded gas budget and can fail when the refundAddress is a smart contract. Whenever the refundAddress either fails to implement the payable fallback function or cumulative gas cost of the function sequence invoked on a native token transfer exceeds 2300 gas consumption limit the native tokens sent end up undelivered and the corresponding user funds return functionality will fail each time.

Proof of Concept

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

Tools Used

None

Recommended Mitigation Steps

Using low-level call.value(amount) with the corresponding result check or using the OpenZeppelin Address.sendValue is advised: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Address.sol#L60

GalloDaSballo commented 2 years ago

See #203

re1ro commented 2 years ago

Duplicate of #4