sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

Jaraxxus - address.call{value:x}() should be used instead of payable.transfer() #105

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

Jaraxxus

medium

address.call{value:x}() should be used instead of payable.transfer()

Summary

The protocol uses Solidity’s transfer() instead of call.

Vulnerability Detail

The protocol uses Solidity’s transfer() when transferring ETH to the recipients. This has some notable shortcomings when the recipient is a smart contract, which can render ETH impossible to transfer. Specifically, the transfer will inevitably fail when the smart contract:

Risks of reentrancy stemming from the use of this function can be mitigated by tightly following the "Check-Effects-Interactions" pattern and using OpenZeppelin Contract’s ReentrancyGuard contract.

            payable(callee).transfer(address(this).balance / strain);
            emit Liquidate(repayable0, repayable1, incentive1, priceX128);
        }

Impact

Stated above.

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Borrower.sol#L282-L285

Tool used

Manual Review

Recommendation

Using low-level call.value(amount) with the corresponding result check or using the OpenZeppelin Address.sendValue is advised

Duplicate of #123

sherlock-admin2 commented 1 year ago

1 comment(s) were left on this issue during the judging contest.

MohammedRizwan commented:

Invalid issue based on sherlock rule