012.md

Dancing Cornflower Stallion

Medium

Unsafe usage of approve method for some ERC20s

Summary

The "default" ERC20 behavior expects the approve function to return a boolean. however, some ERC20s on some chains don't return a value (The most popular example is USDT on the Ethereum Mainnet).

Root Cause

Since it is stated in readme that CHAINS and ERC20 tokens will be in use:

Ethereum Mainnet, Polygon PoS, Arbitrum One, Base

we already took special consideration to make sure USDT works with our contracts.

So for non-standard token such as USDT, calling approve will revert because the solmate ERC20 enforce the underlying token return a boolean.

Internal pre-conditions

Standard ERC20s return a boolean on approval: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol#L67

External pre-conditions

USDT on the main net doesn't return a value: https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code

Attack Path

DOS.

Impact

acceptFundsForAcceptBid function will revert , if the principal token is USDT.

principalToken.approve(address(TELLER_V2), _principalAmount);

PoC

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

Mitigation

Use forceApprove instead of approve.