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

pkqs90 - Protocol fails to work with ERC20s that does NOT revert on failure during transfer.

Closed this issue · 0 comments

sherlock-admin4 commented

pkqs90

high

Protocol fails to work with ERC20s that does NOT revert on failure during transfer.

Summary

There are ERC20 tokens that does NOT revert on failure during transfer of transferFrom, but returns a false instead. The protocol fails to work with these kind of tokens, since there are many places that does not check the return value during a transfer.

Vulnerability Detail

An example of not revert-on-transfer-failure token is ZRX.

The contest README states that any tokens compatible with Uniswap V3 should be supported, which includes ZRX: https://info.uniswap.org/#/tokens/0xe41d2489571d322189246dafa5ebde1f4699f498

We are allowing any standard token that would be compatible with Uniswap V3 to work with our codebase, just as was the case for the original audit of TellerV2.sol. The tokens are assumed to be able to work with Uniswap V3.

There are in total 7 places that uses transfer or transferFrom that does not check the return value. All are listed under Code Snippet section of this report. Here I will discuss one of the most important ones.

In LenderCommitmentGroup_Smart.sol#addPrincipalToCommitmentGroup, it calls a transferFrom to transfer the principal tokens from user to pool, but does not check return value. This means users can pass any amount of _amount and doesn't need to own this amount of tokens, and still mint shares. Basically this means users can mint shares for free.

    function addPrincipalToCommitmentGroup(
        uint256 _amount,
        address _sharesRecipient
    ) external returns (uint256 sharesAmount_) {
        //transfers the primary principal token from msg.sender into this contract escrow
        
>       principalToken.transferFrom(msg.sender, address(this), _amount);

        sharesAmount_ = _valueOfUnderlying(_amount, sharesExchangeRate());

        totalPrincipalTokensCommitted += _amount;
        //principalTokensCommittedByLender[msg.sender] += _amount;

        //mint shares equal to _amount and give them to the shares recipient !!!
        poolSharesToken.mint(_sharesRecipient, sharesAmount_);
    }

Impact

Many impacts. One of the most important ones being users can mint LenderCommitmentGroup_Smart pool shares for free.

Code Snippet

Tool used

Manual review

Recommendation

Use safeTransfer and safeTransferFrom from SafeERC20.

Duplicate of #50