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

0 stars 0 forks source link

Native ETH transfer should use `call()` instead of `transfer()` #602

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Migration.sol#L172-L173

Vulnerability details

Impact

It might be impossible for some addresses to receive ETH via transfer() because receiver address might have methods that exceed 2300 gas, ultimately leading to frozen funds.

Proof of Concept

Native transfer() function has a limit of 2300 gas, which might not be enough when the receiving end has gas expensive operations, leading the transaction to revert.

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Migration.sol#L172-L173

        payable(msg.sender).transfer(ethAmount);

Tools Used

Manual Review

Recommended Mitigation Steps

Use call() instead of transfer() and make sure to implement a boolean return check on call()

Other instance: https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Migration.sol#L325-L326

stevennevins commented 2 years ago

Duplicate of #325

HardlyDifficult commented 2 years ago

Duping to #504