The use of .transfer() in Swap.sol may have unintended outcomes on the eth being sent to the receiver. Eth may be irretrievable or undelivered if the msg.sender or feeRecipient is a smart contract. Funds can potentially be lost if;
The smart contract fails to implement the payable fallback function
The fallback function uses more than 2300 gas units
The latter situation may occur in the instance of gas cost changes. The impact would mean that any contracts receiving funds would potentially be unable to retrieve funds from the swap.
Proof of Concept
This issue directly impacts the following lines of code: L257, L173, L158
Examples of similar issues ranked as medium can be found here and here, just search for 'M04'. A detailed explanation of why relying on payable().transfer() may result in unexpected loss of eth can be found here
Tools Used
Manual review
Recommended Mitigation Steps
Re-entrancy has been accounted for in all functions that reference Solidity's transfer() . This has been done by using a re-entrancy guard, therefore, we can rely on msg.sender.call.value(amount)` or using the OpenZeppelin Address.sendValue library
Handle
elprofesor
Vulnerability details
Impact
The use of
.transfer()inSwap.solmay have unintended outcomes on the eth being sent to the receiver. Eth may be irretrievable or undelivered if themsg.senderorfeeRecipientis a smart contract. Funds can potentially be lost if;The latter situation may occur in the instance of gas cost changes. The impact would mean that any contracts receiving funds would potentially be unable to retrieve funds from the swap.
Proof of Concept
This issue directly impacts the following lines of code: L257, L173, L158
Examples of similar issues ranked as medium can be found here and here, just search for 'M04'. A detailed explanation of why relying on
payable().transfer()may result in unexpected loss of eth can be found hereTools Used
Manual review
Recommended Mitigation Steps
Re-entrancy has been accounted for in all functions that reference
Solidity'stransfer(). This has been done by using a re-entrancy guard, therefore, we can rely onmsg.sender.call.value(amount)` or using the OpenZeppelin Address.sendValue library