#37425 [SC-Insight] redeem collateral does not redeem collateral from riskiest trove but wrongly redeem

#37425 [SC-Insight] redeem_collateral does not redeem collateral from riskiest trove but wrongly redeem lowest healthy troves with lowest collateral Ratio

Submitted on Dec 4th 2024 at 14:14:53 UTC by @perseverance for IOP | Fluid Protocol

• Report ID: #37425

• Report Type: Smart Contract

• Report severity: Insight

• Target: https://github.com/Hydrogen-Labs/fluid-protocol/tree/main/contracts/protocol-manager-contract/src/main.sw

• Impacts:

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

Description

Description

Brief/Intro

The redeem_collateral() in protocol_manager can redeem collateral at any time to receive collateral.

https://github.com/Hydrogen-Labs/fluid-protocol/blob/main/contracts/protocol-manager-contract/src/main.sw#L134-L139

#[storage(read, write), payable]
    fn redeem_collateral(
        max_iterations: u64,
        partial_redemption_hint: u64,
        upper_partial_hint: Identity,
        lower_partial_hint: Identity,
    ) 

According to the documentation of Fluid Protocol,

When USDF is redeemed, the collateral provided to the redeemer is allocated from the Trove(s) with the lowest collateral ratio (CR), even if it is above 135%.

When redeemed, the system uses the USDF to repay the riskiest Trove(s) based on the lowest collateral ratio, and transfers the respective amount of collateral from the affected positions to the redeemer.

On the contrary, redemptions do have a positive effect on the total collateralization of the protocol, improving robustness and stability.

So according to the design of the protocol, the redeem_collateral should redeem from the riskiest trove means that the trove with lowest collateral ratio.

The vulnerability

Vulnerability Details

The vulnerability in redeem_collateral() in internal function get_all_assets_info() from line 328-328.

Line 318: Find the last node in sorted_troves for the asset. This is the riskiest trove for the current asset.

Line 326-328: Loop until it finds the current_borrower that is not zero and have current_cr >= MCR = 135%. Then push that current_borrower to the current_borrowers vector at Line 330.

So it means that current_borrowers vector contains that borrower that have collateral ratio is healthy means >= 135%

So notice that in the case, there are troves with Collateral Ratio < 135% that is unhealthy, the system still look for the healhthy troves to redeem.

https://github.com/Hydrogen-Labs/fluid-protocol/blob/main/contracts/protocol-manager-contract/src/main.sw#L313-L333

    fn get_all_assets_info() -> AssetInfo {
        // Removed for simplicity
            while (i < length) {
         
Line 318:   let mut current_borrower = sorted_troves.get_last(asset);
            let mut current_cr = u64::max();
            if (current_borrower != null_identity_address()) {
                current_cr = trove_manager.get_current_icr(current_borrower, price);
            }
         
Line 326:    while (current_borrower != null_identity_address() && current_cr < MCR) {
Line 327:           current_borrower = sorted_troves.get_prev(current_borrower, asset);
Line 328:           current_cr = trove_manager.get_current_icr(current_borrower, price);
            }
         
Line 330:            current_borrowers.push(current_borrower);
            current_crs.push(current_cr);
            i += 1;
        }

       // Removed for simplicity 

   }

After that, at line 161, the contract find the borrower with the lowest CR in the current_borrowers vector to redeem the collateral. At line 177, the contract call trove_manager_contract.redeem_collateral_from_trove to redeem the collateral.

https://github.com/Hydrogen-Labs/fluid-protocol/blob/main/contracts/protocol-manager-contract/src/main.sw#L159-L184

        let mut assets_info = get_all_assets_info();
        let mut remaining_usdf = msg_amount();
Line 161   let (mut current_borrower, mut index) = find_min_borrower(assets_info.current_borrowers, assets_info.current_crs);
        let mut remaining_iterations = max_iterations;

        // Iterate through troves, redeeming collateral until conditions are met
        while (current_borrower != null_identity_address() && remaining_usdf > 0 && remaining_iterations > 0) {
            let contracts_cache = assets_info.asset_contracts.get(index).unwrap();
            let trove_manager_contract = abi(TroveManager, contracts_cache.trove_manager.bits());
            let price = assets_info.prices.get(index).unwrap();
            let mut totals = assets_info.redemption_totals.get(index).unwrap();
            remaining_iterations -= 1;
            let next_user_to_check = sorted_troves.get_prev(current_borrower, contracts_cache.asset_address);

            // Apply pending rewards to ensure up-to-date trove state
            trove_manager_contract.apply_pending_rewards(current_borrower);

            // Attempt to redeem collateral from the current trove
Line 177:       let single_redemption = trove_manager_contract.redeem_collateral_from_trove(
                current_borrower,
                remaining_usdf,
                price,
                partial_redemption_hint,
                upper_partial_hint,
                lower_partial_hint,
            );
    // Remove for simplicity
    }

In summary, the documentation states that when redeem the collateral, the protocol should redeem from the riskiest troves in the system. But in the code, it does not redeem from the riskiest. In the scenario that there are troves that are not healthy, the system still redeem from the healthy troves.

I give an example as the POC:

For asset_0 , price = 10

# Wallet_1 : 

collateral   = 2000 
debt = 10_000
CR = 2 

# Wallet_2 

collateral   = 900 
debt = 5_000
CR = 1.8 

# Wallet_3