#37671 [SC-Critical] CRITICAL-02 / The contract could be permanently locked due to not reseting the boolen lock

Submitted on Dec 12th 2024 at 09:40:40 UTC by @Minato7namikazi for IOP | Fluid Protocol

Report ID: #37671
Report Type: Smart Contract
Report severity: Critical
Target: https://github.com/Hydrogen-Labs/fluid-protocol/tree/main/contracts/trove-manager-contract/src/main.sw
Impacts:
- Protocol insolvency

Description

In the trove-manager-contract/src/main.sw

There is a path in "internal_redeem_collateral_from_trove" function that leads to an early return statement without resetting the reentrancy lock.

This causes the contract to remain permanently locked, effectively freezing the redemption functionality

Details & Impact

The function internal_redeem_collateral_from_trove uses a storage boolean ( lock_internal_redeem_collateral_from_trove ) to prevent reentrancy attacks. The intended pattern is:

At the start of the function, set the lock to true.
Execute the redemption logic.
At the end of the function, set the lock back to false.

This ensures that if the function reverts or completes normally, the lock is reset, and subsequent calls are not blocked.

within this function, there is a conditional branch that can cause an early return before reaching the lock reset :

// ... calculations and checks above ...

if (new_debt < MIN_NET_DEBT) {
    single_redemption_values.cancelled_partial = true;
    return single_redemption_values;
}

// ... rest of the function that eventually resets the lock ...

In this scenario, if new_debt < MIN_NET_DEBT, the function returns early

never reaching the code that set storage.lock_internal_redeem_collateral_from_trove back to false

As a result, the lock remains permanently engaged. Once locked, no subsequent calls that require this lock to be false can proceed, effectively breaking the redemption functionality

Causing a Critical Protocol insolvency

PoC in "trove-manager-contract/tests/failure.rs"

Proof of Concept

#[tokio::test]

async fn fails_with_stuck_redemption_lock() {

let (contracts, _admin, mut wallets) = setup_protocol(5, false, false).await;

  

oracle_abi::set_debug_timestamp(&contracts.asset_contracts[0].oracle, PYTH_TIMESTAMP).await;

pyth_oracle_abi::update_price_feeds(

&contracts.asset_contracts[0].mock_pyth_oracle,

pyth_price_feed(10),

)

.await;

  

let wallet1 = wallets.pop().unwrap();

let wallet2 = wallets.pop().unwrap();

  

// Setup initial balances

let balance = 25_000 * PRECISION;

token_abi::mint_to_id(

&contracts.asset_contracts[0].asset,

balance,

Identity::Address(wallet1.address().into()),

)

.await;

token_abi::mint_to_id(

&contracts.asset_contracts[0].asset,

balance,

Identity::Address(wallet2.address().into()),

)

.await;

  

// Create troves

let borrow_operations_wallet1 = ContractInstance::new(

BorrowOperations::new(

contracts.borrow_operations.contract.contract_id().clone(),

wallet1.clone(),

),

contracts.borrow_operations.implementation_id.clone(),

);

  

let borrow_operations_wallet2 = ContractInstance::new(

BorrowOperations::new(

contracts.borrow_operations.contract.contract_id().clone(),

wallet2.clone(),

),

contracts.borrow_operations.implementation_id.clone(),

);

  

// first trove with debt that will be below MIN_NET_DEBT after partial redemption

borrow_operations_abi::open_trove(

&borrow_operations_wallet1,

&contracts.asset_contracts[0].oracle,