#37283 [SC-Low] Improper Trove Validation Check Allows Low-Cost Griefing Attack to Block Protocol Redemptions

Submitted on Dec 1st 2024 at 16:23:50 UTC by @InquisitorScythe for IOP | Fluid Protocol

• Report ID: #37283

• Report Type: Smart Contract

• Report severity: Low

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

• Impacts:

  • Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)

Description

Brief/Intro

In the internal_close_trove function, there is a check called require_more_than_one_trove_in_system. Under certain special circumstances, this check can prevent legitimate redeem processes from occurring. An attacker can exploit this to block large redemptions through the protocol by a major holder at an extremely low cost, thereby damaging protocol revenue and potentially causing USDF to depeg.

Vulnerability Details

The documentation states "Any USDF holder can redeem for underlying collateral at any time", however, I found this is not true in the code implementation. When there is only one trove for a certain asset and its MCR (Minimum Collateral Ratio) is the lowest globally, any redemption attempt will fail.

Let's look at the code, anyone can called redeem_collateral. This function will call find_min_borrower to find the borrower/trove with minimal collateral ratio[1], and then start to redeem collateral in a loop[2], if redeem is cancel partially, it will break the loop and skip remaining troves.

    #[storage(read, write), payable] 
    fn redeem_collateral(
        max_iterations: u64,
        partial_redemption_hint: u64,
        upper_partial_hint: Identity,
        lower_partial_hint: Identity,
    ) {
        require(
            storage
                .lock_redeem_collateral
                .read() == false,
            "ProtocolManager: Redeem collateral is locked",
        );
        storage.lock_redeem_collateral.write(true);

        require_valid_usdf_id();
        require(
            msg_amount() > 0,
            "ProtocolManager: Redemption amount must be greater than 0",
        );
        let usdf_contract_cache = storage.usdf_token_contract.read();
        let fpt_staking_contract_cache = storage.fpt_staking_contract.read();
        let usdf = abi(SRC3, usdf_contract_cache.bits());
        let sorted_troves = abi(SortedTroves, storage.sorted_troves_contract.read().bits());
        let active_pool = abi(ActivePool, storage.active_pool_contract.read().bits());
        let fpt_staking = abi(FPTStaking, fpt_staking_contract_cache.bits());
        let mut assets_info = get_all_assets_info();
        let mut remaining_usdf = msg_amount();
        let (mut current_borrower, mut index) = find_min_borrower(assets_info.current_borrowers, assets_info.current_crs); // [1]
        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) { // [2]
            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
            let single_redemption = trove_manager_contract.redeem_collateral_from_trove(
                current_borrower,
                remaining_usdf,
                price,
                partial_redemption_hint,
                upper_partial_hint,
                lower_partial_hint,
            );

            // Break if partial redemption was cancelled
            if (single_redemption.cancelled_partial) {
                break; // [3]
            }

redeem_collateral_from_trove -> internal_redeem_collateral_from_trove do the job internally. There are two cases:

1. if single_redemption_values.usdf_lot can cover the trove's debt, the trove can be closed.

2. otherwise, if new_debt < MIN_NET_DEBT, it returns early and marked cancelled_partial=true, which will break the loop in redeem_collateral.

So, if evildoer can open a trove could not be closed normally, any attempt to redeem collateral will not success.

#[storage(read, write)]
fn internal_redeem_collateral_from_trove(
    borrower: Identity,
    max_usdf_amount: u64,
    price: u64,
    partial_redemption_hint: u64,
    upper_partial_hint: Identity,
    lower_partial_hint: Identity,
) -> SingleRedemptionValues {
    // Prevent reentrancy
    require(
        storage
            .lock_internal_redeem_collateral_from_trove
            .read() == false,
        "TroveManager: Internal redeem collateral from trove is locked",
    );
    storage
        .lock_internal_redeem_collateral_from_trove
        .write(true);
    let mut single_redemption_values = SingleRedemptionValues::default();
    let sorted_troves = abi(SortedTroves, storage.sorted_troves_contract.read().into());
    let asset_contract_cache = storage.asset_contract.read();