#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
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.
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.
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
collateral = 160
debt = 1000
CR = 1.6
For Asset_1, price = 10
# Wallet_2
collateral = 2000
debt = 14_500
CR = 1.37
# Wallet_3
collateral = 500
debt = 1000
CR = 5
Now if the price of asset_1 drop 10% and is 9
For Wallet_2
collateral value = 2000*9 = 18_000
debt = 14_500
CR = 1.24
So now this trove is riskiest in the system.
So if users want to redeem the collateral then it should take from this trove by liquidating the collateral of that trove.
It should redeem from the trove with CR = 1.24
But the current code does redeem from wallet 3 of asset_0, then wallet_2 and then wallet_1
Note that this bug can affect the live contract of Fluid Protocol.
So since the system does not take collateral from the riskiest trove, it cause other users to loose his collateral. So if he think that his position is not the lowest CR in the system, he is surprised to see that his collateral was redeemed and lost money.
Impacts
About the severity assessment
Bug Severity: Critical
Impact category:
Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Step 1: Setup the precondition for this bug as described above
Step 2: Call redeem_collateral
Test Code
Test code to demonstrate this bug: https://gist.github.com/Perseverancesuccess2021/a0a584cc51049b585872b1b87028b1a5#file-success_redemptions_many-rs
Replace the test code function proper_multi_collateral_redemption_from_partially_closed below with the Protocol proper_multi_collateral_redemption_from_partially_closed in fluid-protocol\contracts\protocol-manager-contract\tests\success_redemptions_many.rs
The test log showed that the redemptionEvent for wallet_1 and wallet_2 and wallet_3 for asset_0. The collateral of wallet_1 and wallet_2 and wallet_3 was redeemed.
The trove of wallet_2 with asset_1 is unhealthy and is riskiest in the system, but stay untouched.
Test log: https://gist.github.com/Perseverancesuccess2021/a0a584cc51049b585872b1b87028b1a5#file-proper_multi_collateral_redemption_from_partially_closed_241204_1730-log
