Redeem Illuminate before holdings are filled

Summary

Illuminate redeem functions might burn user tokens but return 0 underlying tokens in return if the holdings are not updated yet.

Vulnerability Detail

The redeem functions (redeem, authRedeem, autoRedeem) of Illuminate do not validate that holdings are filled, the only requirement is to call it after the maturity. This leaves a gap when it is called before holdings are updated (holdings can also be updated only after the maturity), the users will lose their ERC5095 tokens without getting anything in return:

    // Get the amount of tokens to be redeemed from the sender
    uint256 amount = token.balanceOf(msg.sender);

    // Calculate how many tokens the user should receive
    uint256 redeemed = (amount * holdings[u][m]) / token.totalSupply();

    // Update holdings of underlying
    holdings[u][m] = holdings[u][m] - redeemed;

    // Burn the user's principal tokens
    token.authBurn(msg.sender, amount);

    // Transfer the original underlying token back to the user
    Safe.transfer(IERC20(u), msg.sender, redeemed);

When holdings[u][m] is 0, user's redeemed amount will be 0 and all the balance of principal tokens burned :(

Impact

I think it is likely that users or automated integrations might try to redeem their tokens right after maturity. An additional risk is with autoRedeem and authRedeem when the redemptions can be performed automatically if the users gave their approvals. The protocol should eliminate the risk of this occurrence and prevent users from accidentally losing their tokens.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L400-L548

Tool used

Manual Review

Recommendation

Either add extra conditions (e.g. holdings[u][m] > 0) or introduce a slippage parameter (min received).

Duplicate of #239

sourabhmarathe commented 1 year ago

Given that the holdings information is publicly available, we don't think this should be considered a high severity bug. In this case, we would classify this as user error - instead we expect users to ensure that the redemption process has been completed prior to engaging in burning their tokens.