code-423n4 / 2024-01-opus-findings

0 stars 0 forks source link

Kill shrine leads to loss of unpudlled exceptional redistributed yangs #100

Closed c4-bot-9 closed 7 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/caretaker.cairo#L288-L299 https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/shrine.cairo#L1403-L1405 https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/shrine.cairo#L2153-L2182 https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/shrine.cairo#L1416-L1431

Vulnerability details

Vulnerability Details

The caretaker module is responsible for shutting down the shrine and facilitating the process for users to reclaim and release their assets. A portion of the collateral, equivalent to the minted debt, can be withdrawn using the reclaim function with yin debt, while another part of the collateral is withdrawable through the release function using yangs. The release of assets is proportionate to the yang amounts. In the event of any outstanding exceptional yang redistribution that hasn't been pulled, the underlying yangs cannot be added to the trove balance after the shrine is terminated, as the charge function would not work. Consequently, after the shrine is shut down, underlying assets for unpulled yangs become locked in the gate contract, and users have no way to withdraw them, resulting in the loss of their assets.

This happens because if a redistribution was exceptional, yangs would be added to the trove balance only by charge function so we need to call charge to add these yangs to trove balance.

as we see charge function calls pull_redistributed_debts_and_yangs to retrieve updated asset balances array, this would add exceptional redistributed yangs to the trove balance. if there is no exceptional redistribution pull_redistributed_debts_and_yangs would return None as assets balacnes so charge function wouldn't change balances. https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/shrine.cairo#L1420-L1431

charge function would return if the shrine is killed so these yangs can't be added to the trove balance while it's part of yang total_supply therefore user would lose underlying assets of this yang (assets get stuck in gate module). https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/shrine.cairo#L1403-L1405

in caretaker module we can release assets via burning yangs, since unpulled yangs isn't added to trove balance therefore they can't be burned so user can't release those assets. https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/caretaker.cairo#L285-L303

Impact

Loss of unpulled exceptional redistributed yangs (principal assets) after the shrine is killed.

Proof of Concept

Consider the following scenario User A opens a trove, deposits 1 WBTC worth 10000 USD, and forges 5000 yin User B has a trove with 1 ETH worth 1000 USD and 900 yin debt Due to a redistribution all debt and yangs of User B get redistributed to User A Since User 1 doesn't have any ETH in the trove, it would being redistributed exceptionally. debt and yang would be added to User A trove in case of any interaction admin shut down the shrine before User A debt and yang is pulled to trove balance. caretaker calculates backing_pct, since total shrine debt is 5900, and total shrine is valued at 11000 USD so 5100 value of assets can be released by burning yangs. Since 1 ETH yang that has been redistributed exceptionally is not added to the trove A balance, he can't release underlying assets related to this yang in this case and it gets locked in the gate module and can't be withdrawn by anyone.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using pull_redistributed_debt_and_yangs to calculate unpulled yangs and adding to trove balance before release.

Assessed type

Other

tserg commented 8 months ago

This is valid - related to https://github.com/lindy-labs/opus_contracts/pull/531.

c4-pre-sort commented 7 months ago

bytes032 marked the issue as insufficient quality report

c4-pre-sort commented 7 months ago

bytes032 marked the issue as primary issue

c4-pre-sort commented 7 months ago

bytes032 marked the issue as sufficient quality report

c4-sponsor commented 7 months ago

tserg (sponsor) confirmed

c4-judge commented 7 months ago

alex-ppg marked issue #202 as primary and marked this issue as a duplicate of 202

c4-judge commented 7 months ago

alex-ppg marked the issue as satisfactory

c4-judge commented 7 months ago

alex-ppg changed the severity to 2 (Med Risk)