Users can interact with some critical functionalities the protocol when paused
Description
Brief/Intro
the admin is allowed to pause the contract or certain collaterals whenever there might be an issue in the system to avoid exploits or for protocol-known reasons, however, users will still be able to withdraw their collateral when the contract markets functionalities are paused.
Vulnerability Details
with the
`pause_collateral_asset()`
`pause()`
The admin is allowed to stop functionalities of the protocol. For instance on the call to supply_collateral() ```sway // ## 3.1 Supply Collateral #[payable, storage(write)] fn supply_collateral() { // Only allow supplying collateral if paused flag is not set require(!storage.pause_config.supply_paused.read(), Error::Paused); // code let asset_id: AssetId = msg_asset_id(); let collateral_configuration = storage.collateral_configurations.get(asset_id).read();//@audit ere require(!collateral_configuration.paused, Error::Paused); // code // Log user supply collateral event log(UserSupplyCollateralEvent { account: caller, asset_id, amount, }); } ``` if ensures that collateral cannot be supplied if the admin pauses the market, however, the opposite is the case for `withdraw_collateral()` ```sway #[payable, storage(write)] fn withdraw_collateral( asset_id: AssetId, amount: u64, price_data_update: PriceDataUpdate, ) { // no checks against withdrawal when the protocol is paused. reentrancy_guard();
// Get the caller's account and calculate the new user and total collateral
let caller = msg_sender().unwrap();
let user_collateral = storage.user_collateral.get((caller, asset_id)).try_read().unwrap_or(0) - amount;
let total_collateral = storage.totals_collateral.get(asset_id).try_read().unwrap_or(0) - amount;
// Update the storage values (total collateral, user collateral)
storage.totals_collateral.insert(asset_id, total_collateral);
storage
.user_collateral
.insert((caller, asset_id), user_collateral);
// Update price data
update_price_feeds_if_necessary_internal(price_data_update);
// Note: no accrue interest, BorrowCollateralFactor < LiquidationCollateralFactor covers small changes
// Check if the user is borrow collateralized
require(is_borrow_collateralized(caller), Error::NotCollateralized);
transfer(caller, asset_id, amount);
// Log user withdraw collateral event
log(UserWithdrawCollateralEvent {
account: caller,
asset_id,
amount,
});
}
```
Mitigation
Apply same checks in `supply_collateral()` for the `withraw_collateral()`
Impact Details
Here, users will be able to bypass the sanctions put by the admin on the market allowing for exploits to continue even tho the contract is paused.
use fuels::accounts::ViewOnlyAccount; use market::PriceDataUpdate; use market_sdk::parse_units;
const AMOUNT_COEFFICIENT: u64 = 10u64.pow(0); const SCALE_6: f64 = 10u64.pow(6) as f64; const SCALE_9: f64 = 10u64.pow(9) as f64; use crate::utils::{setup, TestData}; use fuels::types::{ContractId, U256}; use market::{CollateralConfiguration, PauseConfiguration}; use market_sdk::get_market_config;
