Loss of liquidation compensation assets in absorb

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-01-opus/blob/main/src/core/purger.cairo#L295

Vulnerability details

Impact

In absorb, since shrine.melt is being called after free, caller will end up receiving less compensation than actually intended. It is because in free, while checking user deposits, there are two cases when user has pending redistribution with exception: 1: User's yang 1 has zero balance 2: User's yang 1 has non zero balance

In both cases user's balances are due for an update, if first n yang balances are zero, caller will miss out on compensation from unaccounted yang updates from all yangs due for redistribution until it encounters one with non zero yang balance. When it encounters first non-zero yang balance, it will still miss out on unaccounted fund from that yang as well, but after that it will be okay, since charge() would be called inside seize function which will then update balances for all yangs.

Proof of Concept

This is inside free function, where only after first non-zero yang balance is seize called and redistribution updated for trove and all its yangs. All calls to deposit will return less than actual balance of trove.

            loop {
                match yangs_copy.pop_front() {
                    Option::Some(yang) => {
                        let deposited_yang_amt: Wad = shrine.get_deposit(*yang, trove_id);

                        let freed_asset_amt: u128 = if deposited_yang_amt.is_zero() {
                            0
                        } else {
                            let freed_yang: Wad = wadray::rmul_wr(deposited_yang_amt, percentage_freed);
                            let exit_amt: u128 = sentinel.exit(*yang, recipient, trove_id, freed_yang);
                            shrine.seize(*yang, trove_id, freed_yang);
                            exit_amt
                        };

                        freed_assets.append(AssetBalance { address: *yang, amount: freed_asset_amt });
                    },
                    Option::None => { break; }
                };
            };

Tools Used

VS Code

Recommended Mitigation Steps

absorb function should update order of melt and free

            shrine.melt(absorber.contract_address, trove_id, purge_amt);
            let compensation_assets: Span<AssetBalance> = self.free(shrine, trove_id, pct_value_to_compensate, caller);

Assessed type

Other

[tserg]

This is valid, and mitigated here.

[c4-pre-sort]

bytes032 marked the issue as primary issue

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-sponsor]

tserg (sponsor) confirmed

[c4-sponsor]

tserg marked the issue as disagree with severity

[tserg]

Caller still gets compensation, plus it is capped to 50 USD, so the impact of this is capped to 50 USD. User funds are also not at risk.

[alex-ppg]

The Warden has showcased a way in which the compensation for the caller of the absorption will be lower than intended.

While the issue is valid, I agree with the Sponsor in that the severity of this cannot be considered high-risk. User funds are indeed affected, however, so I believe a medium-risk rating is more apt.

[c4-judge]

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

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as selected for report