Skip to content
This repository has been archived by the owner on Nov 26, 2023. It is now read-only.

dacian - Use safeTransfer() instead of transfer() for ERC20 tokens #39

Closed
sherlock-admin opened this issue May 5, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented May 5, 2023

dacian

medium

Use safeTransfer() instead of transfer() for ERC20 tokens

Summary

Project explicitly aims to be used with arbitrary ERC20 tokens, hence must use safeTransfer() instead of transfer(). Also doesn't check the return of transfer() either which could be problematic!

Vulnerability Detail

Using transfer() will have problems with non-standard ERC20 tokens.

Impact

Potential issues with non-standard ERC20 tokens and not checking the return of transfer() could be problematic if transfer() fails but doesn't revert & code continues executing as if it had succeeded.

Code Snippet

FootiumEscrow.transferERC20() L110
FootiumPrizeDistributor.claimERC20Prize() L130

Tool used

Manual Review

Recommendation

Use safeTransfer

Duplicate of #86

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 10, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant