sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

bareli - Result of transfer / transferFrom not checked #118

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 3 months ago

bareli

medium

Result of transfer / transferFrom not checked

Summary

no check on the transfer call in withdraw function.

Vulnerability Detail

function withdraw(address payable _to) external onlyCreator returns (uint256) { uint balance = address(this).balance; _to.transfer(balance); return balance; }

Impact

A call to transferFrom or transfer is frequently done without checking the results. For certain ERC20 tokens, if insufficient tokens are present, no revert occurs but a result of "false" is returned. So its important to check this. If you don't you could mint tokens without have received sufficient tokens to do so. So you could loose funds.

Its also a best practice to check this. See below for example where the result isn't checked.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-rng-witnet/src/Requestor.sol#L38

Tool used

Manual Review

Recommendation

Always check the result of transferFrom and transfer. use safetransfer instead of transfer.

nevillehuang commented 2 months ago

Invalid, native ETH transfer does not return boolean values