Illuminate contest - poirots's results

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L192-L235

Vulnerability details

The first implementation of lend, targeting the Illuminate and Yield tokens, is accounting fees when minting tokens for msg.sender, but is not updating fees.

Impact

Lending of Illuminate and Yield tokens, as long as feenumerator > 0, will result in the intended fee amount remaining locked in the Lender contract, and not accounted for when withdrawing fees to the admin. The only way then to retrieve them is via withdraw which always withdraws the full balance.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add the missing logic to this particular lend function:

fees[u] += calculateFee(a)

no emitted events on admin functions

Several functions that set contract settings to do emit events regarding those updates. For future-proofing and outside visibility, it is recommended that events are added to reflect the internal state updates of the contracts

• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L129
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L137
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L145
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L156
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/redeemer/Redeemer.sol#L62
• https://github.com/code-423n4/2022-06- illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/redeemer/Redeemer.sol#L70
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/redeemer/Redeemer.sol#L81
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/redeemer/Redeemer.sol#L92

Safe.sol should document the commit ref of solmate it is inspired from

This can help future developments, and audits, quickly identify possible problems that are fixed in future solmate versions without having to manually check the history

2**256 - 1 can be replaced by type(uint256).max, which is both more readable and more gas-efficient

https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L84

i++ in various for loops , whilei is already being incremented in an unchecked block, you can get further savings by using i += 1 instead of i++

• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L96
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L120
• https://github.com/code-423n4/2022-06-illuminate/blob/3ca41a9f529980b17fdc67baf8cbee5a8035afab/lender/Lender.sol#L289