059.md

Clever Mocha Pheasant

Medium

Share Preparation Time Lock Bypass Through State Overwrite

Summary

The _prepareSharesForBurn function's implementation creates a race condition where existing share preparations can be maliciously overwritten, effectively bypassing the mandatory withdrawal delay period intended to protect against flash loan attacks and market manipulation.

The issue lies in the function's naive state management approach where it directly overwrites the poolSharesPreparedToWithdrawForLender mapping without validating the temporal constraints of existing preparations. This implementation flaw allows an attacker to reset their withdrawal timer at will by submitting new preparations, circumventing the core security mechanism of the protocol's withdrawal system.

This vulnerability undermines the fundamental security assumptions of the withdrawal delay mechanism. An attacker could exploit this to perform rapid withdrawals that should be time-locked, potentially leading to market manipulation through flash loan attacks or front-running opportunities.

The vulnerability exists in the following implementation:

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

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

To demonstrate the exploit, consider the following attack scenario:

// Initial state: Attacker has 1000 shares
// Time: T0
await lenderShares.prepareSharesForBurn(attacker, 1000);

// Time: T0 + 12 hours (half of delay period)
// Withdrawal should still be locked for 12 more hours
// However, attacker can reset timer:
await lenderShares.prepareSharesForBurn(attacker, 1);

// Previous 1000 share preparation is wiped out
// New 24-hour period starts for just 1 share
// Original shares are now free from time lock

Through this mechanism, an attacker can continuously reset their withdrawal timer while maintaining control over their shares, effectively nullifying the protocol's time-lock protection mechanism.

Recommended mitigation steps

The vulnerability should be addressed by implementing proper state management and temporal validation in the preparation process. The following implementation provides a comprehensive fix:

error PreparationStillPending(uint256 timeRemaining);

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

    uint256 existingPrepared = poolSharesPreparedToWithdrawForLender[_recipient];
    if (existingPrepared > 0) {
        uint256 timeElapsed = block.timestamp - poolSharesPreparedTimestamp[_recipient];
        if (timeElapsed < withdrawDelayTimeSeconds) {
            revert PreparationStillPending(withdrawDelayTimeSeconds - timeElapsed);
        }
        
        // Reset expired preparation before accepting new one
        poolSharesPreparedToWithdrawForLender[_recipient] = 0;
        poolSharesPreparedTimestamp[_recipient] = 0;
    }

    poolSharesPreparedToWithdrawForLender[_recipient] = _amountPoolSharesTokens;
    poolSharesPreparedTimestamp[_recipient] = block.timestamp;
    
    emit SharesPrepared(_recipient, _amountPoolSharesTokens, block.timestamp);
    return true;
}

This solution enforces strict temporal validation of preparation states and provides clear error reporting through custom errors. Additionally, it's recommended to implement a cancellation mechanism to allow users to explicitly void their preparations if needed, rather than relying on overwrites:

function cancelSharesPreparation(address _recipient) external onlyOwner {
    require(poolSharesPreparedToWithdrawForLender[_recipient] > 0, "No active preparation");
    
    poolSharesPreparedToWithdrawForLender[_recipient] = 0;
    poolSharesPreparedTimestamp[_recipient] = 0;
    
    emit SharesPreparationCancelled(_recipient);
}

These changes ensure the withdrawal delay mechanism maintains its integrity while providing better user control and visibility into the preparation process.