inflation attack by token transfer

Summary

A malicious St contract creator can steal funds from depositors by launching a typical inflation attack. To execute the attack, the creator can first deposit 1 wei to get 1 wei of ownership. Creator can subsequently send a large amount of collateral directly to the LenderCommitmentGroup_Smart contract - this will hugely inflate the value of the single share.

Now, all subsequent pet owners who deposit their collateral will get no ownership in return. The addPrincipalToCommitmentGroup function uses _valueOfUnderlying(_amount, sharesExchangeRate()); to calculate the ownership of a new depositor. While the total ownership is represented by function _valueOfUnderlying remains the same 1 wei, the totalValueBefore is a huge number, thanks to a large direct deposit done by the creator. This ensures that the 1 wei of share represents a huge value of collateral & causes the ownership of new depositors to round to 0.

Vulnerability Detail

Bob, a malicious actor, initiates the enderCommitmentGroup_Smar contract. By calling addPrincipalToCommitmentGroup, Bob creates a pet depositing a mere 1 wei, which grants him 1 wei of ownership. Bob then directly transfers a significant amount, like 10 ether, to the contract. Consequently, a single 1 wei share becomes equivalent to 10 ether. An innocent user, Pete, tries to create a pet by calling and deposits 1 ether. Pete, unfortunately, receives zero ownership while his deposit remains within the addPrincipalToCommitmentGroup contract

function sharesExchangeRate() public view virtual returns (uint256 rate_) {

    uint256 poolTotalEstimatedValue = getPoolTotalEstimatedValue();

    if (poolSharesToken.totalSupply() == 0) {
        return EXCHANGE_RATE_EXPANSION_FACTOR; // 1 to 1 for first swap
    }

    rate_ =
        (poolTotalEstimatedValue  *
            EXCHANGE_RATE_EXPANSION_FACTOR) /
        poolSharesToken.totalSupply();
}

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); }

function _valueOfUnderlying(uint256 amount, uint256 rate)
    internal
    pure
    returns (uint256 value_)
{
    if (rate == 0) {
        return 0;
    }

@>>> value_ = (amount * EXCHANGE_RATE_EXPANSION_FACTOR) / rate; }

Impact

otential complete loss of funds for new depositors, given they receive no ownership in exchange for their deposited tokens.

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#L321

Tool used

Manual Review

Recommendation

inflation attacks have known defences. A comprehensive discussion can be found here.

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

bareli - inflation attack by token transfer #201