Admin can set very low or very high value for setting fees & collateral factor in `Market.sol` resulting in value loss and/or DoS

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/cc281e5800d5860c816138980f08b84225e430fe/src/Market.sol#L36

Vulnerability details

Proof of Concept

The contract’s admin has control to set values in setCollateralFactorBps(), setLiquidationFactorBps(), setReplenismentIncentiveBps(), setLiquidationIncentiveBps(), setLiquidationFeeBps(). All of them have upper bounds, most of them have lower bounds, but they are mostly serving a sanity check role, they aren’t really upper/lower bounds.

For example admin can set the liquidationIncentive to 99.99% which will result in almost all of the user liquidated collateral going to the liquidator. This is not expected by protocol users and can be seen as an unexpected loss of value for them.

Another issue is admin can set collateralFactorBps to zero, which will DoS withdrawals for users, because of this check in getWithdrawalLimit()

if(collateralFactorBps == 0) return 0;

and since on withdraw you have the following check

uint limit = getWithdrawalLimitInternal(from);
require(limit >= amount, "Insufficient withdrawal limit");

then all withdraws will result in a revert (DoS)

Impact

The admin has too much control over setting protocol params and a malicious or a compromised owner can make it so that users can’t withdraw any of their assets or that they get almost all of their liquidated collateral lost, resulting in a value loss for them.

Recommendation

Add sensible upper & lower bounds for all bps field setters, don’t let collateralFactorBps be set to zero.

[c4-judge]

0xean marked the issue as duplicate

[Simon-Busch]

Issue marked as satisfactory as requested by 0xean

[c4-judge]

Simon-Busch marked the issue as duplicate of #301