sherlock-audit / 2024-04-teller-finance-judging

10 stars 9 forks source link

0xadrii - Using transferFrom won’t work with some tokens #227

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

0xadrii

medium

Using transferFrom won’t work with some tokens

Summary

Using ERC20’s transfer and transferFrom will lead to issues.

Vulnerability Detail

Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/ transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert. Moreover, thetransfer/ transferFrom functions return a boolean value indicating success for tokens that don’t revert. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Impact

Medium. The contract won’t work with certain tokens (for example, USDT), and failed transfers of tokens will still be counted as correct transfers.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L313

Tool used

Manual Review

Recommendation

Use the SafeERC20 to perform ERC20 token transfers, just like in the TellerV2 contract.

Duplicate of #50