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

0 stars 0 forks source link

An attacker could manipulate debt exceptional redistribution because it is allowed to deposit into any trove #115

Open c4-bot-3 opened 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-opus/blob/04583e0411dbf8027952d668a8678fda0cb5b160/src/core/abbot.cairo#L191-L200

Vulnerability details

Impact

In a redistribution, the unhealthy trove's collateral and debt is distributed among troves proportionally to its collateral composition. If no other troves has deposited a yang that is to be redistributed, then the debt and value attributed to that yang will be redistributed among all other yangs in the system according to their value proportionally to the total value of all remaining yangs in the system.

However, the Opus protocol allows anyone to deposit into any trove, even if they are not the trove's owner, as seen in the function abbot.deposit(). This feature could potentially be exploited by an attacker. For instance, if a yang is only deposited in one trove that's being redistributed, the attacker could deposit a small amount into a victim's trove. This could result in all bad debt being redistributed to the victim's trove instead of exceptional redistribution, even if the victim didn't want this (i.e., they didn't deposit this yang into their trove).

fn deposit(ref self: ContractState, trove_id: u64, yang_asset: AssetBalance) {
    // There is no need to check the yang address is non-zero because the
    // Sentinel does not allow a zero address yang to be added.

    assert(trove_id != 0, 'ABB: Trove ID cannot be 0');
    assert(trove_id <= self.troves_count.read(), 'ABB: Non-existent trove');
    // note that caller does not need to be the trove's owner to deposit
    // @audit Attacker could deposit for any trove to make it the only trove deposited this yang and being redistributed when bad debt happen
    self.deposit_helper(trove_id, get_caller_address(), yang_asset);
}

Proof of Concept

Consider the following scenario:

  1. Assume there is a highly volatile yang X added to Opus. Alice, aware of the risk of X, does not deposit any amount of X into her trove.
  2. Bob, the attacker, has deposited some X into his trove.
  3. When Bob's trove is about to be redistributed due to bad debt, since no other trove has deposited X, the debt should be redistributed among all other yangs (exceptional redistribution). This means everyone would share the loss of the bad debt. However, Bob could bypass this by depositing some X into Alice's trove. Since the deposit function doesn't require the caller to be the trove's owner, Alice's trove now holds some X. As a result, a regular redistribution will occur, and all of Bob's bad debt will be redistributed to Alice's trove.

Tools Used

Manual Review

Recommended Mitigation Steps

Limit deposits to only the trove's owner.

Assessed type

Other

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

bytes032 commented 7 months ago

Note for judge re dup #109

Even though both of them are about anyone could deposit to any trove, #109 talked about different case where the threshold of a trove depending on which yangs it deposited,

tserg commented 7 months ago

For #115, it is Acknowledge + Disagree with severity because the likelihood of this happening is low.

For #109, it is Confirm.

c4-sponsor commented 7 months ago

tserg (sponsor) acknowledged

c4-sponsor commented 7 months ago

tserg marked the issue as disagree with severity

alex-ppg commented 7 months ago

The Warden has demonstrated how bad debt re-allocation can be "gamed" due to the permissionless deposit system of troves.

This particular attack vector is quite interesting and I commend the Warden for their out-of-the-box thinking. I believe a medium-risk grade, however, is better suited for it given that the likelihood of a liquidation of a trove with an asset that is not held by any other trove in the system is low.

c4-judge commented 7 months ago

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

c4-judge commented 7 months ago

alex-ppg marked the issue as satisfactory

c4-judge commented 7 months ago

alex-ppg marked the issue as selected for report