sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

thisvishalsingh - thisvishalsingh - Arbitrary from passed to YieldToken::transferFrom lead to loss of funds. #20

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

thisvishalsingh

medium

thisvishalsingh - Arbitrary from passed to YieldToken::transferFrom lead to loss of funds.


thisvishalsingh

title: "Arbitrary from passed to YieldToken::transferFrom lead to loss of funds." labels: "High"

Summary

Not all ERC20 tokens are compliant to the EIP20 standard. Some do not return boolean flag, some do not revert on failure. Use safeTransferFrom consistently instead of transferFrom

Vulnerability Detail

Some [weird-erc20] tokens do not revert on failure, but instead return false (e.g. ZRX).

https://github.com/d-xo/weird-erc20/#no-revert-on-failure YieldToken::transferfrom is directly used to send tokens. If the token send fails, it will cause a lot of serious problems.

Impact

Passing an arbitrary from address to transferFrom (or safeTransferFrom) can lead to loss of funds, because anyone can transfer tokens from the from address if an approval is made.

Code Snippet

            return super.transferFrom(from, to, amount);

Tool used

Manual Review

Recommendation

Consider using safeTransferFrom consistently.

this-vishalsingh commented 7 months ago

@sherlock-admin Without any reason how you can close it? proof me wrong