Venus - RiskOracle Integration

Note that any centralization risks present in the existing codebase before the PR's in scope of this audit were not considered. Only those added to the in-scope PRs are addressed. We recommend all users carefully review the centralization risks, much of which can be found in our previous audits, which can be found here: [https://skynet.certik.com/projects/venus](https://skynet.certik.com/projects/venus). **MarketCapsRiskSteward** In the contract `MarketCapsRiskSteward`, the `DEFAULT_ADMIN_ROLE` of the `AccessControlManager` can grant addresses the privilege to call the function `setMaxDeltaBps()`. Any compromise to the `DEFAULT_ADMIN_ROLE` or accounts granted this privilege may allow the hacker to take advantage of this authority and set the max delta bps to a small non-zero value to prevent updates from process or to a large value to allow malicious updates to successfully process. --- In the contract `MarketCapsRiskSteward`, the `RISK_STEWARD_RECEIVER` has the privilege to call the function `processUpdate()`. Any compromise to the `RISK_STEWARD_RECEIVER` may allow the hacker to take advantage of this authority to process malicious supply and borrow cap updates. --- **RiskStewardReceiver** In the contract `MarketCapsRiskSteward`, the `DEFAULT_ADMIN_ROLE` of the `AccessControlManager` can grant addresses the privilege to call the following functions: - `pause()` - `unpause()` - `setRiskParameterConfig()` - `toggleConfigActive()` Any compromise to the `DEFAULT_ADMIN_ROLE` or accounts granted this privilege may allow the hacker to take advantage of this authority and do the following: - Pause the contract to cause a denial of service. - Unpause the contract to allow actions to be performed when they are not expected. - Set the risk parameter configuration for any update type, where they can set the `riskSteward` to any address and `debounce` to any non-zero value. This could be used to bypass important checks and to allow updates to be processed in quick succession. - Toggle a configuration for an update type. This can cause a denial of service if it is toggled off unexpectedly or allow an unintended configuration to be used if it is toggled on. --- In addition, some of the aliased functions in the adjustment to the core comptroller were privileged functions. The entities that have access to the original privileged function also have access to the aliased privileged function. However, as the functions are simply aliases, they hold the same power.

The contracts `MarketCapsRiskSteward` and `RiskStewardReceiver` are upgradeable; the corresponding `admin` role in each respective proxy has the authority to update the implementation contract behind each contract. Any compromise to the `admin` account in each proxy may allow a hacker to take advantage of this authority and change the implementation contract the proxy points to, and therefore execute potential malicious functionality in the implementation contract.

**RiskStewardReceiver** - `constructor()` and `initialize()` do not have NatSpec comments. - The NatSpec comments for `setRiskParameterConfig()` do not include the `riskSteward` parameter, the event emitted, or the potential errors. - The NatSpec comments for `toggleConfigActive()` do not include the access control, events, or errors. - The NatSpec comments for `processUpdateById()` include the errors `UpdateNotInRange` and `UnsupportedUpdateType`. However, these errors are emitted by the `riskSteward` contract being called and may not always be emitted if new `riskStewards` are utilized. - The NatSpec comments for `processUpdateByParameterAndMarket()` do not include the events or errors. - The functions `_processUpdate()`, `_getMarketUpdateTypeKey()`, and `_validateUpdateStatus()` do not have NatSpec comments. **MarketCapsRiskSteward** - `constructor()` and `initialize()` do not have NatSpec comments. - The NatSpec comments for `setMaxDeltaBps()` do not include the event. - The NatSpec comments for `processUpdate()` do not include the potential events it may emit. - The NatSpec comments for `_updateWithinAllowedRange()` do not include the error. - The functions `_updateSupplyCaps()`, `_updateBorrowCaps()`, `_processSupplyCapUpdate()`, `_processBorrowCapUpdate()`, `_validateSupplyCapUpdate()`, `_validateBorrowCapUpdate()`, and `_decodeBytesToUint256()` are missing NatSpec comments. We assume that all functions should include NatSpec comments. which should include a description of the function, parameters, return values, errors, events, and access control. Please let us know if any if there is another convention you are following in terms of what NatSpec comments will be included for each function. **PolicyFacet** - The NatSpec comments for the function `getBorrowingPower()` do not include the return values. **SetterFacet** - The NatSpec comments for the function `setPrimeToken()` do not include the return value. - The NatSpec comments for the parameter `vTokens` of the function `setMarketSupplyCaps()` reference changing borrow caps, when it should reference changing supply caps. - The functions `__setPriceOracle()`, `__setCloseFactor()`, `__setCollateralFactor()`, `__setLiquidationIncentive()`, `__setMarketBorrowCaps()`, `__setMarketSupplyCaps()`, `__setPrimeToken()`, `__setForcedLiquidation()`, and `__setActionsPaused()` do not have NatSpec comments.

If the changes in scope of this audit are made to update the Core Comptroller Interface to be compatible with the Isolated Pools Interface prior to deploying the risk steward contracts, then the `IIsolatedPoolsComptroller` interface can be used for all comptrollers allowing the cited logic to be simplified.