Contract fails to deliver promised returns, but doesn't lose value
Description
Brief/Intro
The `Market` contract allows adding allowed collateral configuration by the owner of the protocol. Once collateral is added it cannot be removed (just paused). Certain protocol function loop through all collateral configurations regardless if they are paused or not. Once the amount of collateral configurations reaches certain threshold the loop that is going through all the collateral configurations will be too large and will hit the Fuel TX gas limit stopping the protocol operation.
Vulnerability Details
The following `Market` methods are looping through all the collateral configurations:
`get_collateral_configurations()`
`get_all_user_collateral()`
`get_all_totals_collateral()`
`available_to_borrow()`
`is_borrow_collateralized()`
`is_liquidatable_internal()`
`absorb_internal()`
Let's take for example the method `withdraw_base()` which internally calls `is_borrow_collateralized()`, inside we can find the following loop: ``` let len = storage.collateral_configurations_keys.len();
while index < len {
[...]
}
``` For each entry in `collateral_configuration_keys` the loop adds exactly `26508` gas to the TX. The Fuel transaction gas limit is `30 000 000`, this means that after adding exactly `1129` collateral configuration the loop becomes unexecutable due to the above TX gas limit.
As there is no method for removing entries from `collateral_configuration_keys`, there will be no way to fix the protocol once the limit is reached.
Impact Details
Once the collateral amount is reached, the protocols `withdraw_base()` will become un-executable and hence all the base assets will be permanently locked.
Another side effect is that as the collateral configurations are added the gas costs of all the methods which need to cycle through the `collateral_configuration_keys` `StorageVec` increases permanently and hence impacts the cost to the users of the protocol.
Despite reaching the limit having grave consequences for the protocol we chose the severity Medium as the limit is quite large and hence difficult to reach in practice.
Solution Proposal
Add method to remove collateral configuration from the `Market` permanently. This will however have an impact on the currently collateralized loans as it may create bad debt.
Alternative solution would be to create a `Vec` holding a list of supplied collateral assets for each user, and when there is a need the protocol will cycle only through the user's individual list of supplied collateral assets.
References
The problematic loop: https://github.com/Swaylend/swaylend-monorepo/blob/4f1491b86b10121b0ffa7ca68149cf4e3c641684/contracts/market/src/main.sw#L1333
Proof of Concept
Proof of Concept
Use the following test to print the gas cost of a withdrawal: ``` #[tokio::test] async fn poc1_test() { let TestData { wallets, bob, bob_account, alice, alice_account, market, assets, usdc, oracle, price_feed_ids, publish_time, prices, usdc_contract, uni, uni_contract, .. } = setup().await;
let price_data_update = PriceDataUpdate {
update_fee: 0,
price_feed_ids,
publish_times: vec![publish_time; assets.len()],
update_data: oracle.create_update_data(&prices).await.unwrap(),
};
// =================================================
// ==================== Step #0 ====================
// 👛 Wallet: Alice 🧛
// 🤙 Call: supply_base
// 💰 Amount: 5000.00 USDC
let alice_supply_amount = parse_units(5000 * AMOUNT_COEFFICIENT, usdc.decimals);
let alice_mint_amount = parse_units(10_000 * AMOUNT_COEFFICIENT, usdc.decimals);
let alice_supply_log_amount = format!("{} USDC", alice_supply_amount as f64 / SCALE_6);
print_case_title(0, "Alice", "supply_base", alice_supply_log_amount.as_str());
println!("💸 Alice + {alice_supply_log_amount}");
usdc_contract
.mint(alice_account, alice_mint_amount)
.await
.unwrap();
let balance = alice.get_asset_balance(&usdc.asset_id).await.unwrap();
assert!(balance == alice_mint_amount);
let alice_supply_res = market
.with_account(&alice)
.await
.unwrap()
.supply_base(usdc.asset_id, alice_supply_amount)
.await;
assert!(alice_supply_res.is_ok());
market.debug_increment_timestamp().await.unwrap();
// =================================================
// ==================== Step #1 ====================
// 👛 Wallet: Bob 🧛
// 🤙 Call: supply_collateral
// 💰 Amount: 40.00 UNI ~ $200.00
let bob_mint_amount = parse_units(50 * AMOUNT_COEFFICIENT, uni.decimals);
let bob_supply_amount = parse_units(40 * AMOUNT_COEFFICIENT, uni.decimals);
let bob_mint_log_amount = format!("{} UNI", bob_mint_amount as f64 / SCALE_9);
print_case_title(1, "Bob", "supply_collateral", bob_mint_log_amount.as_str());
println!("💸 Bob + {bob_mint_log_amount}");
uni_contract
.mint(bob_account, bob_mint_amount)
.await
.unwrap();
let bob_balance = bob.get_asset_balance(&uni.asset_id).await.unwrap();
assert!(bob_balance == bob_mint_amount);
let bob_supply_res = market
.with_account(&bob)
.await
.unwrap()
.supply_collateral(uni.asset_id, bob_supply_amount)
.await;
assert!(bob_supply_res.is_ok());
let bob_user_collateral = market
.get_user_collateral(bob_account, uni.asset_id)
.await
.unwrap()
.value;
assert!(bob_user_collateral == bob_supply_amount);
market.debug_increment_timestamp().await.unwrap();
// =================================================
// ==================== Step #2 ====================
// 👛 Wallet: Bob 🧛
// 🤙 Call: withdraw_base
// 💰 Amount: 150.00 USDC
let amount_to_fail = parse_units(1000 * AMOUNT_COEFFICIENT, usdc.decimals);
let withdraw_base_fail = market
.with_account(&bob)
.await
.unwrap()
.withdraw_base(&[&oracle.instance], amount_to_fail, &price_data_update)
.await;
assert!(withdraw_base_fail.is_err());
let amount = parse_units(70 * AMOUNT_COEFFICIENT, usdc.decimals);
let log_amount = format!("{} USDC", amount as f64 / SCALE_6);
print_case_title(2, "Bob", "withdraw_base", &log_amount.as_str());
let bob_withdraw_res = market
.with_account(&bob)
.await
.unwrap()
.withdraw_base(&[&oracle.instance], amount, &price_data_update)
.await;
assert!(bob_withdraw_res.is_ok());
println!("Gas used for bob_withdraw_res: {}", bob_withdraw_res.unwrap().gas_used);
let balance = bob.get_asset_balance(&usdc.asset_id).await.unwrap();
assert!(balance == amount);
} ```
Token configurations are added by adding more entries into the `tokens.json` file.
With 5 tokens the test lists the following: ``` Gas used for bob_withdraw_res: 187425 ``` With 4 token the test lists the following: ``` Gas used for bob_withdraw_res: 160923 ```
This means that each collateral configuration adds `26508` gas.
