anya - Insufficient Allowance Handling in withdraw Function and redeem in Tranche.sol

[sherlock-admin]

anya

medium

Insufficient Allowance Handling in withdraw Function and redeem in Tranche.sol

Summary

The withdraw function in the provided code snippet does not explicitly check if the user's allowance is sufficient to cover the desired withdrawal amount before proceeding. This could lead to vulnerabilities and unexpected behaviour if the allowance is less than the requested shares.

Vulnerability Detail

• The code updates the allowance only if the caller is not the owner.
• It subtracts the redeemed shares from the allowance without checking if the allowance is sufficient.
• If the allowance is less than the shares being redeemed, the subtraction would result in a negative value for the allowance.

function withdraw(uint256 assets, address receiver, address owner_)
          {
        ...
            uint256 allowed = allowance[owner_][msg.sender];

            if (allowed != type(uint256).max) allowance[owner_][msg.sender] = allowed - shares;
        }

    }

Impact

• Negative Allowances: Negative allowances are not intended behavior and could potentially lead to issues in other functions relying on the allowance values.
• Unexpected Function Behavior: Dependent functions might handle negative allowances incorrectly, leading to unintended results or potential vulnerabilities.

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/Tranche.sol#L208-L230

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/Tranche.sol#L239-L260

Tool used

Manual Review

Recommendation

• Add a check before updating the allowance to ensure it remains non-negative after subtracting the redeemed shares.
• Consider throwing an appropriate error message if the allowance is insufficient, instead of allowing a negative value.
• If appropriate, explore alternative mechanisms for managing allowances that prevent negative values altogether.

function redeem(uint256 shares, address receiver, address owner_)
    public
    override
    notLocked
    notDuringAuction
    returns (uint256 assets)
{
    if (msg.sender != owner_) {
        uint256 allowed = allowance[owner_][msg.sender];

        // Check for insufficient allowance
        if (allowed < shares) revert TrancheErrors.InsufficientAllowance();

        // Update allowance (preventing negative values)
        allowance[owner_][msg.sender] = allowed - shares;
    }

    // ... 
}

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

[nevillehuang]

Invalid, if insufficient allowance is assigned to user withdrawing, allowed-shares will underflow and revert, no explicit checks required