IllIllI - Holders of worthless external PTs can stick other Illuminate PT holders with bad debts

[sherlock-admin]

IllIllI

high

Holders of worthless external PTs can stick other Illuminate PT holders with bad debts

Summary

Holders of worthless external PTs can stick other Illuminate PT holders with bad debts

Vulnerability Detail

Some of the supported external PTs can pause their activity. One such PT, Pendle, not only can pause activity, but can turn on emergency mode where the admin can transfer the underlying tokens to an arbitrary contract for safekeeping until they decide what to do with the funds. The Illuminate code does not handle such cases, and in fact, if the Pendle protocol is in emergency mode, will still allow users to convert their possibly worthless Pendle PTs to Illuminate ones.

While there is a mechanism for the Illuminate admin to pause a market, there's no guarantee that the Illuminate admin will notice the Pendle pause before other users, and even if they do, it's possible that users have automation set up to front-run such pauses for Pendle markets, so that they never are stuck with worthless tokens.

Impact

Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

Other users that deposited principal in the form of external PTs (e.g. by minting Illuminate PTs in order to be pool liquidity providers) that have actual value, will have their shares of available underlying diluted by Pendle PTs that cannot be redeemed. Illuminate PTs are on a per-share basis rather than a one-for-one basis, so the less underlying there is at redemption time, the less underlying every Illuminate PT holder gets.

Code Snippet

There are no checks that the protocol of the external PT is paused or has any value:

// File: src/Lender.sol : Lender.mint()   #1

270        function mint(
271            uint8 p,
272            address u,
273            uint256 m,
274            uint256 a
275        ) external unpaused(u, m, p) returns (bool) {
276            // Fetch the desired principal token
277            address principal = IMarketPlace(marketPlace).token(u, m, p);
278    
279            // Transfer the users principal tokens to the lender contract
280            Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);
281    
282            // Mint the tokens received from the user
283            IERC5095(principalToken(u, m)).authMint(msg.sender, a);
284    
285            emit Mint(p, u, m, a);
286    
287            return true;
288:       }

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L270-L288

Redemptions of Illuminate PTs for underlyings is based on shares of each Illuminate PT's totalSupply() of the available underlying, not the expect underlying total:
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L422
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L464
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L517

Tool used

Manual Review

Recommendation

Ensure that the protocol being used as principal is not paused before allowing minting

[sourabhmarathe]

In the event of insolvency, we expect the admin to pause any principal token using the unpaused modifier to block minting.

[IllIllI000]

@sourabhmarathe there is no guarantee that the admin will be aware of the insolvency and do the manual step of pausing, before automated tools notice and take advantage of the issue

[JTraversa]

This is generally the case with most integrations across most protocols, there is the chance of an atomic attack on multiple protocols preventing the pausing of markets after detection.

So there arent immediately extremely easy solutions, that said specifically we have already implemented the recommended auditor remediation,

Ensure that the protocol being used as principal is not paused before allowing minting

We do ensure that the protocol being used as a principal does not flag the unpaused modifier before any minting.

[IllIllI000]

The recommendation is to check whether the protocol itself is paused, not to check whether Illuminate has its own paused flag set

[Evert0x]

Valid issue but downgrading to medium severity as the conditions are dependent on an external protocol their admin functions.

[JTraversa]

Understood although this presupposes the idea that all of them can even be paused.

Again, im unsure if this is a reasonable request, as you could submit the same exact report for every single sherlock audit and it would be equally valid for every single integration ever?

Further, if there is an attack, the attacker would simply just attack illuminate before the external protocol can be paused, completely bypassing any checks and just leaving normal users paying more gas.

It all just seems kind of unreasonable, especially as you add additional integrations to the stack (e.g. Illuminate -> Swivel -> Euler -> Lido, do we somehow check EACH of these before every transaction?)

[IAm0x52]

Escalate for 1 USDC

Reminder @Evert0x

[sherlock-admin]

Escalate for 1 USDC

Reminder @Evert0x

You've created a valid escalation for 1 USDC!

To remove the escalation from consideration: Delete your comment.
To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.