Incorrect implementation of Illuminate redeem

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L107-L128

Vulnerability details

Redeeming iPT via redeem will always fail. A griefer can burn token from any holder.

Calling Illuminate redeem is supposed to burn iPT and transfer back the underlying token to the user. However, there are several issues with the current implementation:

uint256 amount = token.balanceOf(msg.sender);

...

// Burn the prinicipal token from Illuminate
token.burn(o, amount);

// Transfer the original underlying token back to the user
Safe.transferFrom(IERC20(u), lender, address(this), amount);

First, the amount is obtained by querying msg.sender's balance. However, in the next step, it's token from o address that is being burned instead of msg.sender.

The final line should be a transfer of underlying token from Redeemer contract to the user. Instead it tries to transfer underlying token from Lender to this contract, which will always fail as Lender is not supposed to hold any underlying token.

This issue also opens up the possibility of griefing by sending the underlying token to Lender and calling redeem to burn iPT from any holder.

Proof of Concept

• Bob has 1000 iPT. Alice has a grudge against Bob.
• On maturity, Alice buys 1000 iPT and sends 1000 USDC to Lender, then proceed to call Illuminate redeem with Bob's address as o.
• Redeemer reads Alice's iPT balance (1000), then proceed to burn that amount from Bob's wallet. 1000 USDC in Lender is sent to Redeemer. Alice redeem the 1000 iPT back to USDC.
• Both Alice and Bob are now down 1000 USDC. 1000 USDC is now stuck at Redeemer.

Recommended Mitigation Steps

Julian clarified that the function allows anyone to burn from any address, but the owner of the iPT themselves will receive the underlying tokens. Therefore, the code should be fixed so that it will query o (the token holder) and transfer the underlying token to them:

uint256 amount = token.balanceOf(o);

...

// Burn the prinicipal token from Illuminate
token.burn(o, amount);

// Transfer the original underlying token back to the user
Safe.transfer(IERC20(u), o, amount);

As a side note, allowing anyone to burn and redeem the iPT of any holder could be dangerous in context of composability. Contracts that aren't aware of this mechanism might end up having funds stuck in them. For example, suppose there is a USDC-iPT/ETH Uniswap pool. When the iPT has matured and someone redeemed the iPT in the pool into USDC, the USDC will be stuck forever and LPs of the pool would lose fund.

Consider limiting redeem to msg.sender instead of allowing arbitrary redemption of any iPT holder.

[sourabhmarathe]

Duplicate of #387.