You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on May 26, 2023. It is now read-only.
Illuminate redemptions don't account for protocol pauses/temporary blocklistings
Summary
Illuminate redemptions don't account for protocol pauses/temporary blocklistings
Vulnerability Detail
By the time that Illuminate PTs have reached maturity, it's assumed that all external PTs will have been converted to underlying, so that the pool of combined underlying from the various protocols can be split on a per-Illuminate-PT-share basis. Unfortunately this may not be the case. Some of the protocol PTs that Illuminate supports as principals allow their own admins topause [activity](# https://docs.notional.finance/developer-documentation/on-chain/notional-governance-reference#pauseability), and Illuminate has no way to protect users from redeeming while these protocol pauses are in effect. Unredeemed external PTs contribute zero underlying to the Illuminate PT's underlying balance, and when a user redeemes an Illuminate PT, the PT is burned for its share of what's available, not the total of what could be available in the future.
Impact
Permanent freezing of funds
If a external PT is paused, or its PT is otherwise unable to be redeemed for the full amount when the user requests it, that unredeemed amount of underlying is not claimable (since the user's Illuminate PT is burned), and the user loses that amount of principal. If the external PT is later able to be redeemed, the remaining users will be given the principal that should have gon to the original user.
Code Snippet
Holdings only increase when external PTs are redeemed successfully:
// File: src/Redeemer.sol : Redeemer.redeem() #1325// Calculate how much underlying was redeemed326uint256 redeemed =IERC20(u).balanceOf(address(this)) - starting;
327328// Update the holding for this market329: holdings[u][m] = holdings[u][m] + redeemed;
And user redemptions of Illuminate PTs does not rely on external PT balances, only on the shares of what's available in the currently stored holdings balance at any point after maturity:
// File: src/Redeemer.sol : Redeemer.redeem() #3413// Verify the token has matured414if (block.timestamp< token.maturity()) {
415revertException(7, block.timestamp, m, address(0), address(0));
416 }
417418// Get the amount of tokens to be redeemed from the sender419uint256 amount = token.balanceOf(msg.sender);
420421// Calculate how many tokens the user should receive422 @>uint256 redeemed = (amount * holdings[u][m]) / token.totalSupply();
423424// Update holdings of underlying425 holdings[u][m] = holdings[u][m] - redeemed;
426427// Burn the user's principal tokens428: token.authBurn(msg.sender, amount);
// File: src/Redeemer.sol : Redeemer.autoRedeem() #5485function autoRedeem(
486addressu,
487uint256m,
488address[] calldataf489 ) externalreturns (uint256) {
490// Get the principal token for the given market491IERC5095 pt =IERC5095(IMarketPlace(marketPlace).token(u, m, 0));
492493// Make sure the market has matured494uint256 maturity = pt.maturity();
495if (block.timestamp< maturity) {
496revertException(7, maturity, 0, address(0), address(0));
497 }
...
514uint256 amount = pt.balanceOf(f[i]);
515516// Calculate how many tokens the user should receive517 @>uint256 redeemed = (amount * holdings[u][m]) / pt.totalSupply();
518519// Calculate the fees to be received (currently .025%)520uint256 fee = redeemed / feenominator;
521522// Verify allowance523if (allowance < amount) {
524revertException(20, allowance, amount, address(0), address(0));
525 }
526527// Burn the tokens from the user528: pt.authBurn(f[i], amount);
The Illuminate admin has no way to pause/disable redemption for users that try to redeem via ERC5095.redeem()/withdraw() or via autoRedeem().
autoRedeem() doesn't use the unpaused modifier, and does not rely on the normal redeem() for redemption:
// File: src/Redeemer.sol : Redeemer.u #6485function autoRedeem(
486addressu,
487uint256m,
488address[] calldataf489 @> ) externalreturns (uint256) {
490// Get the principal token for the given market491IERC5095 pt =IERC5095(IMarketPlace(marketPlace).token(u, m, 0));
492493// Make sure the market has matured494uint256 maturity = pt.maturity();
495if (block.timestamp< maturity) {
496: revertException(7, maturity, 0, address(0), address(0));
The ERC5095 also does not use the unpaused modifier. It uses authRedeem() for its post-maturity redemptions (the pre-maturity redemptions also are not pausable)...:
...and authRedeem() does not use the modifier either:
// File: src/Redeemer.sol : Redeemer.authRedeem() #8443function authRedeem(
444addressu,
445uint256m,
446addressf,
447addresst,
448uint256a449 )
450external451 @>authorized(IMarketPlace(marketPlace).token(u, m, 0))
452returns (uint256)
453 {
454// Get the principal token for the given market455IERC5095 pt =IERC5095(IMarketPlace(marketPlace).token(u, m, 0));
456457// Make sure the market has matured458uint256 maturity = pt.maturity();
459if (block.timestamp< maturity) {
460revertException(7, maturity, 0, address(0), address(0));
461: }
This is hard to solve without missing corner cases, because each external PT may have its own idosyncratic reasons for delays, and there may be losses/slippage involved when redeeming for underlying. I believe the only way that wouldn't allow griefing, would be to track the number of external PTs of each type that were deposited for minting Illuminate PTs on a per-market basis, and require() that the number of each that have been redeemed equals the minting count, before allowing the redemption of any Illuminate PTs for that market. You would also need an administrator override that bypasses this check for specific external PTs of specific maturities. All of this assumes that none of the external PTs have rebasing functionality. Also, add the unpaused modifier to both Redeemer.autoRedeem() and Redeemer.authRedeem().
The text was updated successfully, but these errors were encountered:
Unsure if this is being used as the main ticket for the authRedeem/autoRedeem when paused, as that issue is mentioned alongside the primary description of external PTs that may have been paused, preventing redemption in time for iPT maturity.
If this ticket is primarily regarding the latter, i'd probably contest and claim that we would have ample time between external PT maturity and our own maturity to pause ourselves and remediate any issues should there be external pauses.
This current implementation of manual pausing saves significant gas when compared to the suggested remediation given the additional storage required for individual market checks, so as long as we are not irresponsible / our keepers are operating, there are no additional risks introduced here that are remediated by checking balances and requiring 1:1 amounts.
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.
IllIllI
high
Illuminate redemptions don't account for protocol pauses/temporary blocklistings
Summary
Illuminate redemptions don't account for protocol pauses/temporary blocklistings
Vulnerability Detail
By the time that Illuminate PTs have reached maturity, it's assumed that all external PTs will have been converted to underlying, so that the pool of combined underlying from the various protocols can be split on a per-Illuminate-PT-share basis. Unfortunately this may not be the case. Some of the protocol PTs that Illuminate supports as principals allow their own admins to pause [activity](# https://docs.notional.finance/developer-documentation/on-chain/notional-governance-reference#pauseability), and Illuminate has no way to protect users from redeeming while these protocol pauses are in effect. Unredeemed external PTs contribute zero underlying to the Illuminate PT's underlying balance, and when a user redeemes an Illuminate PT, the PT is burned for its share of what's available, not the total of what could be available in the future.
Impact
Permanent freezing of funds
If a external PT is paused, or its PT is otherwise unable to be redeemed for the full amount when the user requests it, that unredeemed amount of underlying is not claimable (since the user's Illuminate PT is burned), and the user loses that amount of principal. If the external PT is later able to be redeemed, the remaining users will be given the principal that should have gon to the original user.
Code Snippet
Holdings only increase when external PTs are redeemed successfully:
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L325-L329
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L385-L394
And user redemptions of Illuminate PTs does not rely on external PT balances, only on the shares of what's available in the currently stored holdings balance at any point after maturity:
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L413-L428
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L449-L470
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L485-L528
The Illuminate admin has no way to pause/disable redemption for users that try to redeem via
ERC5095.redeem()/withdraw()
or viaautoRedeem()
.autoRedeem()
doesn't use theunpaused
modifier, and does not rely on the normalredeem()
for redemption:https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L485-L496
The ERC5095 also does not use the
unpaused
modifier. It usesauthRedeem()
for its post-maturity redemptions (the pre-maturity redemptions also are not pausable)...:https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L284-L345
...and
authRedeem()
does not use the modifier either:https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L443-L461
Tool used
Manual Review
Recommendation
This is hard to solve without missing corner cases, because each external PT may have its own idosyncratic reasons for delays, and there may be losses/slippage involved when redeeming for underlying. I believe the only way that wouldn't allow griefing, would be to track the number of external PTs of each type that were deposited for minting Illuminate PTs on a per-market basis, and
require()
that the number of each that have been redeemed equals the minting count, before allowing the redemption of any Illuminate PTs for that market. You would also need an administrator override that bypasses this check for specific external PTs of specific maturities. All of this assumes that none of the external PTs have rebasing functionality. Also, add theunpaused
modifier to bothRedeemer.autoRedeem()
andRedeemer.authRedeem()
.The text was updated successfully, but these errors were encountered: