Illuminate contest - Picodes's results

Lines of code

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

Vulnerability details

Impact

Funds may be stuck when redeeming for Illuminate.

Proof of Concept

Assuming the goal of calling redeem for Illuminate here is to redeem the Illuminate principal held by the lender or the redeemer, then there is an issue because the wrong balance is checked. So if no msg.sender has a positive balance funds will be lost.

Now assuming the goal of calling redeem for Illuminate here is for users to redeem their Illuminate principal and receive the underlying as suggested by this comment, then the underlying is not sent back to users because Safe.transferFrom(IERC20(u), lender, address(this), amount); send the funds to the redeemer, not the user.

Recommended Mitigation Steps

Clarify the purpose of this function and fix the corresponding bug.

Lines of code

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

Vulnerability details

Impact

When redeeming from APWine and Tempus here, the principal token is not transferred to the redeemer, so may be stuck on the lender contract.

Proof of Concept

Instead of Safe.transferFrom(IERC20(u), lender, address(this), amount); it should be Safe.transferFrom(IERC20(principal), lender, address(this), amount);

[NC - 01] - Readability could be improved when taking max value of a type

Here, since Solidity 0.8.8, you can use type(uint256).max.

Check https://github.com/ethereum/solidity/blob/develop/Changelog.md#088-2021-09-27

[NC - 02] - Typo

Here, avaialable -> available

[NC - 03] - Typo

Here, prinicipal -> principal

[NC - 04] - Use readable variable names

The code interact with various protocols and is already quite complicated, no need to obfuscate it with unreadable variable names such as u, n, m,

[L - 01] - Redeem does not behave as explained in the comments

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

When redeeming Illuminate's token, the underlying is not sent back to the user, like said in the previous comment. So reading this function code is misleading and may lead to a user calling it.