Did not Approve to Zero First

Summary

Allowance was not set to zero first before changing the allowance.

Vulnerability Detail

Some tokens (e.g. USDT, KNC) do not allow approving an amount M > 0 when an existing amount N > 0 is already approved. This is to protect from an ERC20 attack vector described here.

The following attempts change the allowance without setting the allowance to zero first:

function redeem(uint256 shares, address recipient, address owner) public returns (uint256 amount) {
    if (shares == type(uint256).max) shares = maxRedeem(owner);

    if (msg.sender != owner) {
        uint256 allowed = allowance[owner][msg.sender];
@>      if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares;
    }

Lender.sol - Line 182

function approve(address spender, uint256 shares) external returns (bool) {
@>  allowance[msg.sender][spender] = shares;

Lender.sol - Line 322

function transferFrom(address from, address to, uint256 shares) external returns (bool) {
    uint256 allowed = allowance[from][msg.sender];
@>  if (allowed != type(uint256).max) allowance[from][msg.sender] = allowed - shares;

Lender.sol - Line 337

@>  allowance[recoveredAddress][spender] = value;

Lender.sol - Line 388

Impact

However, if the token involved is an ERC20 token that does not work when changing the allowance from an existing non-zero allowance value, it will break all of these key functions or features of the protocol.

Code Snippet

Provided Above

Tool used

Manual Review

Recommendation

It is recommended to set the allowance to zero before increasing the allowance or use safeApprove/safeIncreaseAllowance.

sherlock-audit / 2023-10-aloe-judging

alymurtazamemon - Did not Approve to Zero First #58