Venus Protocol security record

The `ResilientOracle` contract attempts to manage extreme scenarios where a substantial number of dependent oracles fail. Nonetheless, the oracle function used does not return the last update time, meaning that the risk of utilizing stale price data still remains with a low likelihood. We also note, the oracle also assumes that the `ResilientOracle` will accurately return the price of a `WETH` in terms of USD. In case this assumption fails, the oracle will return inaccurate results.

The `getPrice()` function accepts a parameter but reverts unless it is `WSTETH_ADDRESS`. In this case the parameter is not necessary, however, may still be kept in order to conform to the `OracleInterface`.

The in-scope contract is compiled using Solidity version 0.8.13. According to the [Solidity official's compiler bug list](https://docs.soliditylang.org/en/latest/bugs.html), this version contains several known compiler bugs, specifically 1 medium/high-severity, 2 medium-severity, and 6 low-severity bugs. According to our examination, these bugs seem to not affect the code in scope.

The protocol is planned to be deployed on Arbitrum and opBNB. Both chains allow users to send cross-chain messages to call L2 contracts from L1 so that the interaction with L2 protocols can continue even if the sequencer is down. However, when an L2 contract receives a cross-chain call from L1, the `msg.sender` value is usually not the sender who initiates the cross-chain call on L1. On Arbitrum, the `msg.sender` is the [aliased address](https://docs.arbitrum.io/arbos/l1-to-l2-messaging#address-aliasing) of the original sender. On opBNB, which is built on the OP Stack, the `msg.sender` can be the [L2 messenger contract](https://community.optimism.io/docs/developers/bridge/messaging/#accessing-msg-sender) or an [aliased address](https://community.optimism.io/docs/developers/bedrock/differences/#deposits-from-the-underlying-l1-ethereum-goerli-etc-to-l2-op-mainnet-op-goerli-etc), depending on which L1 contract is called and whether the original sender is a contract or not. Therefore, access control mechanisms based on the `msg.sender` value should take such changes into account if a privileged role needs to call an L2 contract from L1 in certain circumstances, for example, in an emergency but directly sending L2 transaction is not possible since the sequencer is down. In the past, the Uniswap Factory deployed on Arbitrum did not consider address aliasing, and its owner was set to the address of the Timelock contract on L1, causing the execution of governance proposals on Arbitrum to be temporarily blocked ([reference](https://gov.uniswap.org/t/consensus-check-fix-the-cross-chain-messaging-bridge-on-arbitrum/18547)).

The `BaseXVSProxyOFT` contract takes `sharedDecimals_` as a constructor parameter and sets the `ld2sdRate`. According to the code comments in `OFTCoreV2`, > _sharedDecimals should be the minimum decimals on all chains The XVS tokens will have 18 decimals on each EVM chain. However, it should be noted that there is an upper limit of `type(uint64).max` on `sd` amounts. This is required for bridging to non-EVM chains such as Aptos and Solana. If `sharedDecimals_` is set to 18, at most ~18 XVS tokens can be bridged in each call. As mentioned by the Layer Zero docs, ["OFTV2 is intended to be used with no more than 10 shared decimals"](https://layerzero.gitbook.io/docs/evm-guides/layerzero-omnichain-contracts/oft/oft-v1-vs-oftv2-which-should-i-use#what-are-the-differences-between-the-two-versions).

The XVS vault internal mapping `rewardTokenAmountsPerBlock`, as the name implies, stores the amount of rewards per block for each registered token. On chains with a fixed block time, since each block is created (approximately) periodically, this implies “per unit of time”. Therefore, as seen in the table below, special care needs to be taken when setting this value in the different chains (these values can and might change in the [future](https://etherscan.io/chart/blocktime)). A wrong value could result in unexpected rewards being provided to users. For chains with irregular block times, please refer to the other related issues. | Chain | Avg Blocktime [s] | |----------|:-----------------:| | Ethereum | 12 | | BSC | 3 | | Optimism | 2 | | opBNB | 1 | | Arbitrum | ~12 (L1), Irregular (L2) | | zkEVM | Irregular | <br> **References:** * [opBNB vs other Layer2 Networks | BNB Optimistic Rollup](https://docs.bnbchain.org/opbnb-docs/docs/core-concepts/difference-L2) * [Arbitrum Block Numbers and Time](https://docs.arbitrum.io/time)

In the function `AccessControlManager.giveCallPermission()`, the event `PermissionGranted` is bypassable by computing the `role` off-chain and calling the function `AccessControlManager.grantRole()` directly. Similarly, the event `PermissionRevoked` is bypassable by calling `AccessControlManager.revokeRole()` directly.

The contract `AccessControlManager` allows the `DEFAULT_ADMIN_ROLE` to grant an account access to a specified function accross **all** managed contracts by passing the zero address when calling `AccessControlManager.giveCallPermission()`. While code comments indicate that this is the intended design, it is crucial to note that such a design *could* lead to uninteded access when a new contract is managed by the `AccessControlManager`, or when a currently managed contract is upgraded with new functions.

The `ChainlinkOracle.getPrice()` and `_getPriceInternal()` functions fetch the given ERC-20 token's `decimal` and multiply the asset price by `10 ** (18 - decimal)`. However, this approach cannot handle ERC-20 tokens with decimals larger than 18, as the subtraction will trigger an integer overflow.

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. In this protocol, access control is also granted on a per-function basis through `_checkAccessAllowed()`. **VTreasuryV8** The `VTreasuryV8.sol` contract contains the following privileged roles: 1. An owner (`owner`, `onlyOwner` modifier) as initialized during the constructor execution to `msg.sender`: 1. Transfer any amount of any token to any address by calling `withdrawTreasuryToken()`. 2. Transfer any amount of the native token to any address by calling `withdrawTreasuryNative()`. <br> **XVS Bridge & XVS** The `BaseXVSProxyOFT.sol` contract contains the following privileged roles: 1. An owner (`owner`, `onlyOwner` modifier) as initialized during the constructor execution to `msg.sender`: 1. `setOracle()`: Set the address of the `ResilientOracle` contract. 1. `setMaxSingleTransactionLimit()`: Sets the limit of a single transaction amount. 1. `setMaxDailyLimit()`: Sets the limit of daily (24-hour) transactions amount. 1. `setMaxSingleReceiveTransactionLimit()`: Sets the maximum limit for a single receive transaction. 1. `setMaxDailyReceiveLimit()`: Sets the maximum daily limit for receiving transactions. 1. `setWhitelist()`: Sets the whitelist address to skip checks on transaction limit. 2. `pause()`: Pauses the bridge. 3. `unpause()`: Unpauses the bridge. <br> The `XVSBridgeAdmin.sol` contract contains the following privileged roles: 1. An owner (`owner`, `onlyOwner` modifier) as initialized during the constructor execution to `msg.sender`: 1. Insert or modify the mapping between function signatures and names to allow specific roles to call the privileged functions on the bridges. <br> The `XVS.sol` contract contains the following privileged roles: 1. An Access Control Manager (`accessControlManager_`), as initialized during the constructor execution to `accessControlManager_`: 1. Control who is allowed to mint tokens by indirectly affecting `_ensureAllowed("mint(address,uint256)")`. 2. Control who is allowed to burn tokens by indirectly affecting `_ensureAllowed("burn(address,uint256)")`. <br> The following functions in this contract are protected through `_ensureAllowed()`: 1. `mint(address,uint256)`: Mints a capped `amount_` of tokens to a specified `account_`. 2. `burn(address,uint256)`: Burns any `amount_` of tokens from a specified `account_`. <br> **XVS Vault** The `XVSVault.sol` contract contains the following privileged roles: 1. An administrator (`admin`, `onlyAdmin()` modifier), as initialized during the constructor execution to `msg.sender`: 1. Change the XVS store contract address (`xvsStore`) that controls reward token transfers by calling `setXvsStore()`. 2. Change the access control manager implementation address (and thereby indirectly all `_checkAccessAllowed()`-controlled access checks) by calling `setAccessControl()`. <br> The following functions in this contract are protected through `_checkAccessAllowed()` with their corresponding function signatures: 1. `pause()`: Pause the contract (and thereby all calls to `deposit()`, `claim()`, `executeWithdrawal()`, `requestWithdrawal()`, `updatePool()`, `delegate()` and `delegateBySig()`) by calling `pause()`. 1. `resume()`: Resume the contract from a paused state by calling `resume()`. 1. `add(address,uint256,address,uint256,uint256)`: Add a new pool by calling `add()`. 1. Using an existing reward token, but a new pool token allows the caller to globally update the `rewardTokenAmountsPerBlock[]` variable for all pools using this reward token, **without explicitly calling `setRewardAmountPerBlock()`**. In extreme cases, this could either drain the reward token (by choosing a high reward value) or halt any future rewards (by choosing zero). 1. **Accidentally or with malicious intent the caller could add enough new pools to make calling `massUpdatePools()` prohibitively gas-expensive and thereby deny the service to the following functions**: 1. `add()` 1. `set()` 1. `setRewardAmountPerBlock()` 1. `set(address,uint256,uint256)`: Update the accumulated rewards per share for all pools and change the allocation point of a pool by calling `set()`. 1. `setRewardAmountPerBlock(address,uint256)`: Update the accumulated rewards per share for all pools and arbitrarily change the reward amount per block by calling `setRewardAmountPerBlock()`. 1. **By setting it to zero no additional rewards would be accumulated.** 1. **By setting it to an unusually high value the reward token could be drained.** 1. `setWithdrawalLockingPeriod(address,uint256,uint256)`: Change the lockup period for withdrawing deposited funds by calling `setWithdrawalLockingPeriod()`. <br> The `XVSStore.sol` contract contains the following privileged roles: 1. An administrator (`admin`, `onlyAdmin()` modifier), as initialized during the constructor execution to `