search

sherlock-audit / 2023-07-kyber-swap-judging

12 stars 8 forks source link

Mahi_Vasisth - Use _safetransfer() instead of _transfer() #67

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Mahi_Vasisth

medium

Use _safetransfer() instead of _transfer()

Summary

The use of _transfer() in the function for transferring funds cannot guarantee the successful transfer of funds. This is because _transfer() always returns a true or false value for the transaction and does not revert the function in case of an unsuccessful transfer.

Vulnerability Detail

The _transfer() function always returns a boolean value (true or false) for the transaction. If the transaction fails for any reason, it will return false but will not revert the function.

Impact

This can result in funds not being transferred to the destination address while allowing the function to execute completely.

Code Snippet

https://github.com/sherlock-audit/2023-07-kyber-swap/blob/main/ks-elastic-sc/contracts/Pool.sol#L617

Tool used

Manual review

Recommendation

It is recommended to replace _transfer() with _safetransfer() to ensure safer and more reliable fund transfers. _safetransfer() is designed to revert the transaction in case of failure, providing better assurance of successful fund transfers.

sherlock-admin commented 1 year ago

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

Trumpero commented:

invalid, spam issue. This _transfer doesn't return bools, it is internal function of ERC20