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] fnredeem_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());letmut assets_info =get_all_assets_info();letmut remaining_usdf =msg_amount();let (mut current_borrower, mut index) =find_min_borrower(assets_info.current_borrowers, assets_info.current_crs); // [1]letmut remaining_iterations = max_iterations;// Iterate through troves, redeeming collateral until conditions are metwhile (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();letmut 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 trovelet 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 cancelledif (single_redemption.cancelled_partial) {break; // [3] }
redeem_collateral_from_trove -> internal_redeem_collateral_from_trove do the job internally. There are two cases:
if single_redemption_values.usdf_lot can cover the trove's debt, the trove can be closed.
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)]fninternal_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 reentrancyrequire( 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);letmut single_redemption_values =SingleRedemptionValues::default();let sorted_troves =abi(SortedTroves, storage.sorted_troves_contract.read().into());let asset_contract_cache = storage.asset_contract.read();// Get the trove details for the borrowerlet trove = storage.troves.get(borrower).read();// Calculate the amount of USDF to redeem (capped by max_usdf_amount or trove's debt) single_redemption_values.usdf_lot =fm_min(max_usdf_amount, trove.debt);// Calculate the corresponding amount of asset to redeem based on the current price single_redemption_values.asset_lot =fm_multiply_ratio(single_redemption_values.usdf_lot, DECIMAL_PRECISION, price);// Calculate the new debt and collateral amounts after redemptionlet new_debt = trove.debt - single_redemption_values.usdf_lot;let new_coll = trove.coll - single_redemption_values.asset_lot;// If the trove's debt is fully redeemed, close the troveif (new_debt ==0) {internal_remove_stake(borrower);internal_close_trove(borrower, Status::ClosedByRedemption);internal_redeem_close_trove(borrower, 0, new_coll); } else {// Calculate the new nominal collateralization ratiolet new_nicr =fm_compute_nominal_cr(new_coll, new_debt);// If the new debt is below the minimum allowed, cancel the partial redemptionif (new_debt < MIN_NET_DEBT) { single_redemption_values.cancelled_partial =true;return single_redemption_values; }// Re-insert the trove into the sorted list with its new NICR sorted_troves.re_insert( borrower, new_nicr, upper_partial_hint, lower_partial_hint, asset_contract_cache, );// Update the trove's debt and collateral in storageletmut trove = storage.troves.get(borrower).read(); trove.debt = new_debt; trove.coll = new_coll; storage.troves.insert(borrower, trove);// Update the stake and total stakesinternal_update_stake_and_total_stakes(borrower); }
in internal_close_trove, there is a check called require_more_than_one_trove_in_system. Instead of checking whether more than one trove in the protocol as it suggested, it only check for one asset.
#[storage(read)]fnrequire_more_than_one_trove_in_system( trove_owner_array_length:u64, asset_contract:AssetId, sorted_troves_contract:ContractId,) {let sorted_troves =abi(SortedTroves, sorted_troves_contract.into());let size = sorted_troves.get_size(asset_contract);require( trove_owner_array_length >1&& size >1,"TroveManager: There is only one trove in the system", );}
So here is the situation when all redemption can be blocked.
User A wants to redeem a large amount of USDF, say 100k, through the fluid protocol.
Evildoer E can always front run and block the redemption with following step: a. Identify the asset with no trove opened, let say xxETH is this case. b. Open a trove with globally minimal cr, and only slightly larger than MIN_NET_DEBT; e.g., if price of asset xxETH is 1. then evildoer can create a debt of 500 USDF with 679 xxETH, then he has a trove with 135.12% cr.
User A can't redeem 100k, because internal_redeem_collateral_from_trove falls into situation 1, but the trove could not be closed. Reverted by require_more_than_one_trove_in_system.
User B wants to redeem a small amount of USDF, say 100. It is also not possible. Because internal_redeem_collateral_from_trove falls into situation 2, the redeem loop breaks early.
Notice that, even xxETH has already some troves open, the evildoer can front run to redeem those troves, only left one trove to block the redemption.
Impact Details
Let's talk about the impact.
Evildoer can always front run to block redemption with relatively low cost: The redemption no longer depends on the overall staking depth, but rather on the number of stakes in a specific asset. In a situation where there are no troves for a particular asset, an evildoer only needs to pay 500*0.5%=2.5 USDF as an opening fee to prevent redemption operations across the entire protocol.
The prevention of redemption is hard to recover: In cases where certain assets lack liquidity, this deadlock situation is very difficult to break.
The prevention of redemption is harmful for the robustness and stability of protocol, could leads to USDF depeg: With the loss of the redemption channel that was promised in the initial protocol design, USDF's credibility will suffer, liquidity will decrease, and ultimately lead to depegging consequences.
The fix suggestion is remove require_more_than_one_trove_in_system checks, or implement it across all assets.
References
Add any relevant links to documentation or code
Proof of Concept
Create a new tes file in contracts/protocol-manager-contract/tests/test_redemptions.rs
run cargo test -- --nocapture failed_redemption_case1, the output shows that redemption revert, even there are enough collateral for the requested 1000 amount.
run cargo test -- --nocapture failed_redemption_case2, this time wallet1 try to redeem 100 USDF. The output shows that, though it did not revert, redeemer wallet1's balance did not change at all. The redemption failed without error message.
