[WP-M5] `liquidation_queue` Lack of input validation for `safe_ratio` can disrupt liquidation if misconfigured

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/money-market-contracts/contracts/liquidation_queue/src/contract.rs#L169-L171

Vulnerability details

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/money-market-contracts/contracts/liquidation_queue/src/contract.rs#L169-L171

if let Some(safe_ratio) = safe_ratio {
    config.safe_ratio = safe_ratio;
}

When update_config(), the value of safe_ratio is not being validated. The owner can set it to a value > 1 and it will disrupt liquidation and leads to bad debt of the protocol.

overseer#liquidate_collateral() will call query_liquidation_amount():

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/money-market-contracts/contracts/overseer/src/collateral.rs#L156-L165

let liquidation_amount_res: LiquidationAmountResponse = query_liquidation_amount(
    deps.as_ref(),
    deps.api.addr_humanize(&config.liquidation_contract)?,
    borrow_amount,
    borrow_limit,
    &cur_collaterals.to_human(deps.as_ref())?,
    collateral_prices,
)?;

let liquidation_amount = liquidation_amount_res.collaterals.to_raw(deps.as_ref())?;

When config.safe_ratio is set to a value higher than 1, the safe_borrow_amount can be larger than borrow_amount, as a result, borrow_amount - safe_borrow_amount can revert on underflow at L301.

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/money-market-contracts/contracts/liquidation/src/contract.rs#L295-L306

// When collaterals_value is smaller than liquidation_threshold,
// liquidate all collaterals
let safe_borrow_amount = borrow_limit * config.safe_ratio;
let liquidation_ratio = if collaterals_value < config.liquidation_threshold {
    Decimal256::from_uint256(borrow_amount) / Decimal256::from_uint256(expected_repay_amount)
} else {
    Decimal256::from_uint256(borrow_amount - safe_borrow_amount)
        / Decimal256::from_uint256(expected_repay_amount - safe_borrow_amount)
};

// Cap the liquidation_ratio to 1
let liquidation_ratio = std::cmp::min(Decimal256::one(), liquidation_ratio);

Recommendation

Consider adding validation to make sure safeRatio is always less than or equal to 1.

[GalloDaSballo]

Looks like admin privilege due to lack of validation

[albertchon]

Downgrading to QA as it relies on botched admin initialization, which is unlikely