Skip to content

Latest commit

 

History

History
110 lines (77 loc) · 5.44 KB

017.md

File metadata and controls

110 lines (77 loc) · 5.44 KB

Bubbly Inky Sawfish

High

Title: Incorrect implementation of LenderCommitmentGroupShares._afterTokenTransfer() may lead to DoS of LenderCommitmentGroup_Smart

Summary

Shareholders of LenderCommitmentGroupShares must call prepareSharesForBurn in advance to burn their shares. There is a cooldown mechanism in place for burning of their shares LenderCommitmentGroupShares. This mechanism utilizes the LenderCommitmentGroupShares._afterTokenTransfer() function to reset poolSharesPreparedToWithdrawForLender and poolSharesPreparedTimestamp each time a transfer or burn occurs. As a result, the holders of LenderCommitmentGroupShares cannot burn their shares for withdrawDelayTimeSeconds after any transfer or burn action. However, due to absence of a non-zero check in the LenderCommitmentGroupShares._afterTokenTransfer() function, anyone can reset poolSharesPreparedToWithdrawForLender and poolSharesPreparedTimestamp of other share holders by calling transferFrom() with a zero amount. This vulnerability allows attackers to capture all SharesPrepared events, effectively creating a DoS situation for burning shares. Consequently, while all shareholders can mint shares, they are unable to burn them, resulting in all funds staked in LenderCommitmentGroup_Smart being locked indefinitely.

Root Cause

Shareholders of LenderCommitmentGroupShares must call prepareSharesForBurn in advance to burn their shares. https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroupShares.sol#L99-L118

     function _prepareSharesForBurn(
        address _recipient,
        uint256 _amountPoolSharesTokens 
    ) internal returns (bool) {
   
        require(  balanceOf(_recipient) >= _amountPoolSharesTokens  );

        poolSharesPreparedToWithdrawForLender[_recipient] = _amountPoolSharesTokens; 
        poolSharesPreparedTimestamp[_recipient] = block.timestamp; 


        emit SharesPrepared(  
            _recipient,
            _amountPoolSharesTokens,
           block.timestamp

         );

        return true; 
    }

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

    function burn(address _burner, uint256 _amount, uint256 withdrawDelayTimeSeconds) external onlyOwner {


        //require prepared 
@>      require(poolSharesPreparedToWithdrawForLender[_burner] >= _amount,"Shares not prepared for withdraw");
@>      require(poolSharesPreparedTimestamp[_burner] <= block.timestamp - withdrawDelayTimeSeconds,"Shares not prepared for withdraw");
        
 
        //reset prepared   
        poolSharesPreparedToWithdrawForLender[_burner] = 0;
        poolSharesPreparedTimestamp[_burner] =  block.timestamp;
  

        _burn(_burner, _amount);
    }

There is a cooldown mechanism in place for burning of their shares LenderCommitmentGroupShares. This mechanism utilizes the LenderCommitmentGroupShares._afterTokenTransfer() function to reset poolSharesPreparedToWithdrawForLender and poolSharesPreparedTimestamp each time a transfer or burn occurs. https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroupShares.sol#L60-L71

     function _afterTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal override {


         //reset prepared   
@>      poolSharesPreparedToWithdrawForLender[from] = 0;
@>      poolSharesPreparedTimestamp[from] =  block.timestamp;

    }

Shareholders of LenderCommitmentGroupShares are unable to burn their shares for withdrawDelayTimeSeconds after any transfer or burn action. However, the absence of a non-zero check for the transfer amount allows attackers to reset poolSharesPreparedToWithdrawForLender and poolSharesPreparedTimestamp for any account by calling transferFrom() with a zero amount.

Attackers can create a DoS situation for burning shares by monitoring the SharesPrepared events emitted from LenderCommitmentGroupShares and repeatedly invoking transferFrom(). As a result, shareholders are unable to burn their shares forever.

While the protocol owner could set withdrawDelayTimeSeconds to zero as a workaround, this would introduce another high-severity vulnerability, which is described here.

Internal pre-conditions

none

External pre-conditions

none

Attack Path

  1. Alice monitors all the SharesPrepared events emitted from LenderCommitmentGroupShares.
  2. Each time she captures an event, Alice calls transferFrom with a zero amount.

As a result, shareholders are unable to burn their shares indefinitely.

Impact

All funds deposited in LenderCommitmentGroup_Smart will be permanently locked.

PoC

none

Mitigation

     function _afterTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal override {

+       if (amount == 0) return;
         //reset prepared   
        poolSharesPreparedToWithdrawForLender[from] = 0;
        poolSharesPreparedTimestamp[from] =  block.timestamp;

    }