Critical protocol parameter configuration/changes should have sanity/threshold checks

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

Input validation on key function parameters is a best-practice. Not applying sanity/threshold checks will allow incorrect values to be set accidentally/maliciously and may significantly affect the security/functionality of the protocol.

Except checks on liquidation fees and collaterization factor, the codebase does not have input validation (sanity/threshold checks) on key protocol parameters in setter functions that are callable only by contract owners. Given that the fundamental value proposition of the protocol is to address risk management using isolated lending pairs, it becomes even more important to enforce sanity/threshold validation on protocol parameters to increase confidence in them, reduce risk and prevent accidental/malicious changes that increases risk significantly. The sanity/threshold values may be configurable in a time-delayed manner by owner/governance instead of hardcoding and enforcing unilaterally.

Scenario: Protocol is initialized/configured with absurd/unreasonable (i.e. very low/high) values of critical parameters such as depositLimit, borrowLimit, minBorrowUSD, pool allocation points, totalRewardPerBlock, poolFee, twapPeriod or minObservations. Owners/users fail to check or understand the impact of these absurd values until the protocol’s functionality or profitability is affected significantly.

Proof of Concept

See similar Major-severity finding from Consensys Diligence Audit of Shell Protocol: https://consensys.net/diligence/audits/2020/06/shell-protocol/#certain-functions-lack-input-validation-routines

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/Controller.sol#L123

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/Controller.sol#L130

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/Controller.sol#L137

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/RewardDistribution.sol#L78

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/RewardDistribution.sol#L119

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/RewardDistribution.sol#L132

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/UniswapV3Oracle.sol#L47

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/UniswapV3Oracle.sol#L69

https://github.com/code-423n4/2021-07-wildcredit/blob/82c48d73fd27a9d4d5d4a395b3affcef4ef6c5c8/contracts/UniswapV3Oracle.sol#L73

Tools Used

Manual Analysis

Recommended Mitigation Steps

Enforce sanity/threshold checks in all onlyOwner setters.

[talegift]

Severity should be 1.

[ghoul-sol]

Agree with sponsor, this is low risk at most.