Illuminate contest - Kumpa's results

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L545-L589

Vulnerability details

Impact

An attacker could abuse the lack of check on aave and pool addresses by creating fake aave and APWine pool contracts. The fake aave could transfer deposited underlying token back to the attacker while the IAPWineRouter pool could do nothing but return any amount that the attacker want. This allows the attacker to mint principalToken freely

Proof of Concept

1. Attacker creates fake aave contract with function named deposit that receives underlying token from the Lender.sol and later send it to attacker

2. Attacker creates fake pool contract with function named swapExactAmountIn that does not perform any swap but returns any number that the attacker wants

3. Attacker inputs those addresses in the lend to perform an attack

4. Attacker's underlyingToken will initially get transfered to Lender.sol but after depositing to a fake aave contract, the attacker will get his underlyingToken back

5. fake pool returns any amount of principalToken that the attacker wants

Mitigations

Implementing require to check the eligibility of the external contract before calling

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L102-L124

Vulnerability details

Impact

Without the check on address to be approved, a malicious admin could approve any external contract to get the token that it want. Since the allowance is maximum, a malicious contract could steal as much token as it wants from the contract after the approval.

Proof of Concept

Mitigations

Consider whitelisting the allowed address and not allow the admin to arbitariry approve any address.

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L185-L235

Vulnerability details

Normally the amount of fees after calculateFee should be added into fees[u] so that the admin could withdraw it through withdrawFee. However, illuminate ledning does not track fees[u]. Therefore, the only way to get fees back is through withdraw which admin needs to wait at least 3 days before receiving the fees.

Mitigations

Add the amount of fee after each transaction into fees[u] like other lending method.
for example: fees[u] += calculateFee(a);

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L705-L720 https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L659-L675

Vulnerability details

withdrawFee of eToken requires the amount of eToken in Lender.sol >= fees[eToken] so Safe.transfer will not revert. However if the admin withdraw(eToken) first, the balance of eToken in Lender.sol will equal to zero while fees[eToken] remains the same and withdrawFee(eToken) will become unfunctioning since eToken in the contract does not match fees[eToken]. The admin will need to rely on withdraw, which takes 3 days before transfering, to get the future fees of eToken.

Mitigations

add fees[eToken] = 0; after withdrawals[e] = 0; in withdraw.