Incident
Incident,
5M.
Source: De.Fi incident database.
- Audits before it
- 4.
- Audits since
- 63, the latest dated 13 Sep 2025.
Venus Protocol security record
As of 30 Oct 2026, Venus Protocol has 67 audit reports by 7 security firms on Trustblock; the latest is by Quantstamp, dated 13 Sep 2025. Those reports list no open critical findings. It also holds 2 deployed contracts on 2 chains and 2 incidents (latest 12 May 2022).
Critical findings
No open critical findings
Across the 67 reports, 4 critical findings were reported. Each is marked fixed by the firm that reported it. See the findings
Incident
Incident 12 May 2022
Source: De.Fi incident database. 63 audits have been published since. See the incidents
Open the latest report
Your label, live
Get the label for your siteRecord last updated . It holds audits and findings, contracts and incidents up to Oct 2026. Trustblock does not review code.
Audits67
Newest first
67 audit reports, 40 in CertiK’s series, 11 in Quantstamp’s series
| Audit | Security firm | Date | Scope | Critical findings | Report |
|---|---|---|---|---|---|
| Venus - Multichain Governance | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus wstETH Oracle | Security firm: Quantstamp | Date: | Scope: Repository at commit f638cef | No open critical findings | Open · certificate.quantstamp.com |
| Venus - Token Converter Addendum | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus TWAP Oracle | Security firm: HashEx | Date: | Scope: 1 address on BNB Chain | No open critical findings | Open · hashex.org |
| Venus - Oracle for wstETH on Ethereum | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Time based contracts & SeizeVenus | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - XVS Token Bridge | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Prime and Oracle Changes | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus Multichain Support | Security firm: Quantstamp | Date: | Scope: Repository at commit 5299454 | No open critical findings | Open · certificate.quantstamp.com |
| Venus - Private Conversions | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Prime | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Token Converter | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus Token Converter Audit | Security firm: OpenZeppelin | Date: | Scope: Files named in the report | No open critical findings | Open · blog.openzeppelin.com |
| Venus - Forced liquidations in Isolated pools | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus Income Allocation | Security firm: Quantstamp | Date: | Scope: Repository at commit 2e0f5f6 | No open critical findings | Open · certificate.quantstamp.com |
| Venus - Force Liquidations | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Allocation of Income | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus Protocol Diamond Comptroller Audit | Security firm: OpenZeppelin | Date: | Scope: Files named in the report | No open critical findings | Open · blog.openzeppelin.com |
| Venus - RiskFund & Shortfall | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Isolated Pools Allow Multiple Rewards Dist | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - Diamond Comptroller (Subscription Audit 5) | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus Protocol V2 | Security firm: Quantstamp | Date: | Scope: Repository at commit a9c6e07 | No open critical findings | Open · certificate.quantstamp.com |
| Venus Liquidator | Security firm: Quantstamp | Date: | Scope: Repository at commit 457c8cd | No open critical findings | Open · certificate.quantstamp.com |
| Venus - Oracle Update | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Venus - RewardsDistributor | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
Findings426
Statuses are the security firm’s own, from each report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 3 |
| High | 16 | 0 | 13 |
| Medium | 5 | 4 | 32 |
| Low | 71 | 60 | 222 |
CriticalIncorrect Fee Amount Will Cause All Swaps To FailFixed
The `PancakeLibrary` is used to interact with pairs created through the `PancakeRouter v2` (See [0x10ed43c718714eb63d5aa57b78b54704e256024e](https://bscscan.com/address/0x10ed43c718714eb63d5aa57b78b54704e256024e#code) for deployed code). In `v1` the fee was $0.2\%$, however, the fees were changed in `v2` to be $0.25\%$. The logic here assumes that the fee is only $0.2\%$, when in fact a $0.25\%$ fee will be taken. This means that the calculated `amountIn` will be less than the amount needed and the `amountOut` will be greater than the amount that can be received, causing the constant product to not be preserved and reverting the transaction.
CriticalIncorrect Mathematical OperationFixed
The report gives no description here.
CriticalNo Approval Given For `safeTransferFrom()`Fixed
In the function `swapVAIForStable()`, `safeTransferFrom()` is used to transfer the `stableTknAmountUSD` from `address(this)` to the `receiver`. However, the contract does not give approval to itself, so that this will cause a revert preventing `swapVAIForStable()` from being called successfully.
HighAccess Control ViolationFixed
The report gives no description here.
HighAccess Control ViolationFixed
The report gives no description here.
HighDenial of Service - Loop Gas LimitFixed
The report gives no description here.
HighHighly Permissive Role AccessFixed
The report gives no description here.
HighMissing Swap Path ValidationFixed
The report gives no description here.
HighNon-Finalized CodeFixed
The report gives no description here.
HighRace ConditionFixed
The report gives no description here.
Show 10 more
HighRequirements ViolationFixed
The report gives no description here.
HighRequirements ViolationFixed
The report gives no description here.
HighStakers in XVS Vault May Be Able to Double Their Voting Power by Re-StakingFixed
Users can deposit XVS into the XVS vault to get voting power. However, according to the implementation, when users request a withdrawal, their voting power is removed only when the pool's reward token is XVS. For pools having a different reward token, the user's voting power will not be removed, and therefore they can re-stake to the pool to gain double voting power with the same amount of XVS deposit.
HighUndocumented BehaviorFixed
The report gives no description here.
HighUse of Unsafe Cast Operations Leading to Incorrect Voting PowerFixed
The following functions make use of unsafe cast operations (`uint96()`), exposing them to truncation, when reaching values higher than `type(uint96).max` (`79,228,162,514,264,337,593,543,950,335`): 1. `deposit()`: Depositor could end up with less votes than he is entitled to. 1. `requestWithdrawal()`: Withdrawer is debited a too small amount. 1. `getStakeAmount()`: May return a truncated (too small) staked amount.
HighXVS Vault Users Receive Fewer Rewards if Multiple Pools Have the Same Pool TokenFixed
When updating the rewards for the pools in the XVS vault, `pool.token.balanceOf(address(this))` is used as a pool's `supply` (i.e., the total deposits in the pool). However, if multiple pools have the same type of pool token, using `balanceOf()` returns the total deposits across all the pools instead of a specific one. As a result, the value of `supply` will be overestimated, causing pool users to receive fewer rewards than they should.
MediumFunctions Will Always Return Success Even When ThVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The updates to the functions do not return the value obtained from the internal function call. As a result, they will always return the default value of 0, which is interpreted as success. This can result in assuming that the function was successfully called, when it was not, which can cause issues if further changes to the protocol are made assuming the function call was successful.
Open the reportMediumUpdates May Be Called Out Of Order Or To PreventVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
There are scenarios where a previous update can be processed after a later update, causing the protocol to use earlier updates than desired. In addition, it allows for scenarios where a previous update can be processed to ensure that the latest update will expire before it can be processed.
Open the reportMediumPendle Oracle Does Not Always Return Rate ScaledVenus - PendleOracle Upgrade · CertiK · 26 Dec 2024Fixed
The function `_getUnderlyingAmount()` is designed to get the `underlyingToken` amount for 1 `ptToken` scaled by the `underlyingToken` decimals. However, this function simply returns the value obtained from the `PT_ORACLE`, which does not always have this scaling and can result in an incorrect price being returned. For example lets assume that PT pumpBTC 27MAR2025 is to be supported [0x997Ec6Bf18a30Ef01ed8D9c90718C7726a213527](https://etherscan.io/address/0x997Ec6Bf18a30Ef01ed8D9c90718C7726a213527). If one uses the `PendlePtLpOracle` at address [0x66a1096C6366b2529274dF4f5D8247827fe4CEA8](https://etherscan.io/address/0x66a1096C6366b2529274dF4f5D8247827fe4CEA8) (currently used by Venus) and fetches the rate via the pumpBTC market [0x8098b48a1c4e4080b30a43a7ebc0c87b52f17222](https://etherscan.io/address/0x8098b48a1c4e4080b30a43a7ebc0c87b52f17222), it will return a value with 18 decimals of precision, however WBTC only has 8 decimals. This would result in the wrong price being returned. In our testing when calling `PT_ORACLE.getPtToAssetRate(0x8098b48a1c4e4080b30a43a7ebc0c87b52f17222, 900)` we got a value of `988245041751264715`, which is scaled by 1e18 as opposed to 1e8.
Open the reportMediumPotential Reentrancy AttackVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The function `execute()` does not follow check-effect-interact and the transactions executed may make external calls allowing for potential reentrancy. In particular, if there is more than one proposal queued, then this can allow a second proposal to be executed in the middle of the execution of another proposal. Considering the proposal executed in the middle of the other proposal can execute privileged actions that may not have been accounted for, this could cause potential harm to the protocol.
Open the reportShow 10 more
MediumRounding Is Not In Favor Of The Protocol For UserVenus - Token Converter Addendum · CertiK · 15 Feb 2024Fixed
When calculating the amount of tokenIn for a user to provide for a given amount of tokenOut the logic rounds down. This allows a user to obtain up to `(tokenInToOutConversion - 1) / EXP_SCALE` tokenOut without providing any tokenIn.
Open the reportMediumFunction _increaseMintLimit() Does Not Handle PosVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
Function `_increaseMintLimit()` is used within the `burn()` function of the in-scope `XVS` token contract as a way to document the increase in available funds to mint after burning. The function logic assumes that the `minterToCap` mapping for the source address `from` will be larger than the calculated `totalMintedNew` value after subtracting away the new `amount_` that is burned. If in between minting and burning, the `minterToCap` corresponding value is decreased for the minter address, then it is possible that `minterToCap` is less than the calculated `totalMintedNew` value at the time of burning. In that case, the logic of `_increaseMintLimit()` will cause a revert in the `burn()` logic, preventing burning from functioning properly.
Open the reportMediumMissing Checks for Sequencer Uptime When Fetching Chainlink PricesVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The `ChainlinkOracle._getChainlinkPrice()` function fetches the latest price of an asset from Chainlink price feeds. However, as the protocol is intended to be deployed on Arbitrum and opBNB, checking the sequencer's uptime before querying price data is crucial. If the sequencer is down, there is a chance of using an incorrect or stale price. Please refer to [Chainlink L2 Sequencer Uptime Feeds](https://docs.chain.link/data-feeds/l2-sequencer-feeds) document for more details.
Open the reportMediumPotential of Locked Funds in XVS BridgesVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The function `BaseOFTV2.sendAndCall()` burns or locks XVS tokens from users with the intention of bridging them over to another chain to an address provided by the user. However, the recipient, `_toAddress`, **must** be a contract on the destination chain. Otherwise, the function `OFTCoreV2._sendAndCallAck()` will mint or release tokens through `_creditTo()` without transferring them to the designated address, essentially locking the XVS tokens in the bridge contract. Additionally, if for some reason a failed transaction is unable to be retried through `retryMessage()`, tokens might be unable to be sent to the destination address even though they might have been burned in the source chain.
Open the reportMediumXVS Bridges May Accept Messages from Compromised RemotesVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The XVS bridges are set up as non-blocking LayerZero apps, which allows for the replaying of unsuccessful messages via `NonblockingLzApp.retryMessage()`. `retryMessage()` makes a subsequent call to `OFTCoreV2._nonblockingLzReceive()`, which attempts to retry the failed message. This can be an issue if a trusted remote is compromised and is removed as a trusted remote since neither `retryMessage()` nor `_nonblockingLzReceive()` perform any validation checks on whether the `_srcAddress` on the source chain is still a trusted remote.
Open the reportMediumERC-777 Tokens Lead to Improper TrackingVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportMediumFee-On-Transfer Tokens Lead to Improper TrackingVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportMedium`swapVAIForStable()` Does Not Meet SpecificationFixed
The function `swapVAIForStable()` is supposed to swap VAI for the input `stableTknAmount` of stable Token. However, it swaps VAI for the `stableTknAmountUSD` instead. This does not meet the specification outlined in this proposal: [https://community.venus.io/t/proposal-deploy-the-vai-peg-stability-module-and-supply-liquidity-to-pancakeswap-for-the-vai-usdt-pair/3461](https://community.venus.io/t/proposal-deploy-the-vai-peg-stability-module-and-supply-liquidity-to-pancakeswap-for-the-vai-usdt-pair/3461).
Medium`VAIVault` Is Susceptible to MEV BotsFixed
After the XVS rewards are transferred to the VAI vault, someone has to call `updatePendingRewards()` to make the vault update the internal accounting for pending rewards. Typically, when a user operates (e.g., borrow, redeem) on a vToken, the `Comptroller` transfers the XVS tokens to the VAI vault and calls the update function in the same transaction. An attacker can exploit this design to get most of the rewards by depositing a large number of funds, triggering the reward emission, and then withdrawing their deposit. As a result, they do not have to stake VAI in the vault for some period but can still get rewards from it. The attacker might not be incentivized to launch such an attack if the rewards are relatively small. However, this could be an issue if the accumulated XVS rewards are large enough (e.g., a long period has passed, but no one operates on any vToken). In addition to the MEV exploit, the current implementation could lead to improper accounting. After someone sends the XVS reward to the VAI vault, the `updatePendingRewards()` function has to be executed before any other user operations such as `deposit()` or `withdraw()`. Otherwise, users can lose their eligible rewards, which can be locked in the vault permanently. At the beginning of every user operation, the `updateAndPayOutPending()` function is called, which claims the rewards for the user and calls the `safeXVSTransfer()` function. The bug occurs when the `safeXVSTransfer()` function updates the `xvsBalance` state variable according to the current XVS balance. Since the current XVS balance may include pending rewards that are not accounted for, `xvsBalance` should not be overwritten with the current XVS balance.
MediumBest Practice Violation - Check Effects Interaction Pattern ViolationFixed
The report gives no description here.
Show 10 more
MediumContradiction - Name ContradictionFixed
The report gives no description here.
MediumContradiction - NatSpec Comments ContradictionFixed
The report gives no description here.
MediumContradiction - NatSpec ContradictionFixed
The report gives no description here.
MediumContradiction - Non-Finalized CodeFixed
The report gives no description here.
MediumContradiction - Requirement ContradictionFixed
The report gives no description here.
MediumIf `vToken` Is Added Multiple Times To `pendingReFixed
If the redemption of a `vToken` fails when liquidating, then that `vToken` is added to the `pendingRedeem` array. Thus if multiple liquidations happen with the same `vTokenCollateral` and all redemptions fail, the same `vToken` will be added to the `pendingRedeem` array multiple times. Thus when `_reduceReservesInternal()` is called it will reference the same `vToken` at different indexes. However, this function uses the contracts balance of `vToken`, so that if it redeems for one of the indexes the `vTokenBalance` of the contract will become zero. Then when any of the other indexes are attempted, the redemption will revert as it will have a zero amount. In this way the `pendingRedeem` array will grow with no ability to remove some of its elements, which can be used to prevent the contract from being able to redeem `vToken` and transfer the underlying asset to the protocol share reserve.
MediumInconsistent Data - Missing Event for Critical Value UpdatesFixed
The report gives no description here.
MediumInconsistent Data - Unused Return ValueFixed
The report gives no description here.
MediumInconsistent Data - Variable Is Not LimitedFixed
The report gives no description here.
MediumInefficient Gas ModelFixed
The report gives no description here.
Show 10 more
MediumInefficient Gas Model - Redundant LibraryFixed
The report gives no description here.
MediumInefficient Gas Model - Redundant State ConstantFixed
The report gives no description here.
MediumInitializers Can Be Called More than OnceFixed
`VAIVault.setVenusInfo()` and `XVSVault.setXvsStore()` functions are used to set the different token addresses. However, the admin can call the function again and change the addresses, thereby locking the tokens in the vault.
MediumMishandled Edge CaseFixed
The report gives no description here.
MediumMissing Events EmittingFixed
The report gives no description here.
MediumPossible To Be Unable To Repay Or Liquidate BorroFixed
When VAI is swapped for stable tokens, if `vaiMinted` is zero it will burn VAI that has been borrowed. This can cause the total amount of VAI in circulation to be less than the amount of VAI borrowed, which can make it impossible to repay or liquidate all the borrows. Note that this is possible because if USDT is over peg more VAI is burned than USDT is transferred out and if USDT is under peg less VAI is minted than USDT is transferred in.
MediumRe-Entrancy Guards Bypassable by Admins in `XVSVault.sol` and `VAIVault.sol`Fixed
All three vault contracts `XVSVault.sol`, `VRTVault.sol` and `VAIVault.sol` use a custom re-entrancy guard modifier (`nonReentrant`) (i.e. rather than relying on the [well-established one from OpenZeppelin](https://docs.openzeppelin.com/contracts/4.x/api/security#ReentrancyGuard)). While otherwise it works as expected, it is problematic in `XVSVault.sol`, where it is initialized in function `setXvsStore()`, which however can be called multiple times. The same issue applies to `VAIVault.setVenusInfo()`.
MediumVRT Vault Users Can Earn Additional Interests When Depositing After the Last Accrual BlockFixed
When a user deposits into the VRT vault and `lastAccruingBlock` is behind the current block, their `accrualStartBlockNumber` will be set to `lastAccruingBlock`. Although the user deposits at the current block, their interests are accrued starting from the `lastAccruingBlock`. They can claim those interests once the `lastAccruingBlock` has been moved forward to a more recent block.
Low`VTokenstorage` Does Not Keep Extra SlotsMigration Core (Venus) · Quantstamp · 13 Sep 2025Fixed
The new `VTokenInterfaceR1` implementation now inherits from `VTokenStorageR1` instead of `VTokenStorage`. The previous `VTokenStorage` contract reserved an additional 50 storage slots through a `uint256[50] __gap` placeholder, ensuring room for future variable additions without altering the storage layout. After the migration, this explicit reservation is no longer visible to developers. In subsequent upgrades, they may overlook the previously intended expansion buffer or inadvertently add storage variables beyond the implicit 50-slot allowance, risking storage collisions or corrupting the upgrade path.
Open the reportLow`_configureMarkets()` side effectsVenus - WUSDMLiquidator · CertiK · 29 Apr 2025Fixed
The `run()` function emits multiple events in the context of the Comptroller and different VTokens on every execution. Since `run()` is unprotected and can be invoked repeatedly, this behavior may lead to unexpected and potentially disruptive events. Furthermore, the function presumes that the `VWUSDM` market is paused for `Action.MINT` and `Action.ENTER_MARKET`, and maintains the paused state after execution. This may be unintended if the market gets unpaused in the future.
Open the reportShow 10 more
Low`closeFactorMantissa` is set via Comptroller upgrVenus - WUSDMLiquidator · CertiK · 29 Apr 2025Fixed
```sol=109 uint256 internal constant MAX_CLOSE_FACTOR_MANTISSA = 1e18; // 1.0, temporarily ``` `WUSDMLiquidator` assumes the Comptroller code will be upgraded, allowing `closeFactorMantissa` to be set higher than the regular 0.9e18. It is unclear when and if the `MAX_CLOSE_FACTOR_MANTISSA` will be restored. ```sol=124 COMPTROLLER.setCloseFactor(1e18); ``` `WUSDMLiquidator` uses the 1e18 value directly as a close factor. It's reasonable to use `COMPTROLLER.MAX_CLOSE_FACTOR_MANTISSA()` instead to ensure the call will always be successful. Or `MANTISSA_ONE` constant in case it is supposed to revert in case of unexpected COMPTROLLER behavior.
Open the reportLow`run()` is unprotectedVenus - WUSDMLiquidator · CertiK · 29 Apr 2025Fixed
Despite the [description](https://community.venus.io/t/proposal-partial-liquidity-restoration-for-wusdm-on-venus-zksync/5013) stating that the WUSDMLiquidator is meant to be executed solely by Governance, the `run()` function is unprotected and can be called multiple times by any user.
Open the reportLowTypos And InconsistenciesVenus - asBNB Oracle · CertiK · 20 Mar 2025Fixed
- The comment in the constructor refers to upgradability, when it has been stated previously that proxy contracts will be removed and thus the upgradability of the contracts. If this change will be done prior to the deployment of this contract, then we recommend making the comment consistent. - In other contracts the oracles name starts with the first letter capitalized, however, this is not done for the `asBNBOracle` in the comments or file name.
Open the reportLowUnnecessary ImportVenus - asBNB Oracle · CertiK · 20 Mar 2025Fixed
The contract imports `ensureNonzeroAddress` but does not use it.
Open the reportLow`_getUnderlyingAmount()` Design IntentVenus - ZkETH Oracle · CertiK · 25 Feb 2025Fixed
The function `_getUnderlyingAmount()` calls `LSTPerToken()` on the zkETH contract, which does the following ```solidity function LSTPerToken() external view returns (uint256) { return _getWrappedLiquidStakedTokenStorage().lst.convertToAssets( 1 ether, true ); } ``` Similarly to the wstETH oracle, this could instead call `convertToAssets(1 ether,true);` directly on the rzkETH contract to save gas. However, the zkETH contract is upgradeable while the wstETH contract is not, so that it is possible that the `LSTPerToken()` function's behavior is changed in the future, which is not the case for wstETH.
Open the reportLowAliased Function Not Included In Updated InterfacVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The `setActionsPaused()` function is an aliased version of the `_setActionsPaused()` function. However, it is not included in the updated `ISetterFacet` interface.
Open the reportLowDisabled Function Does Not Emit An ErrorVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The function `renounceOwnership()` is disabled and does not emit an error. This can lead to confusion if it is called, as the call will succeed but do nothing.
Open the reportLowEvent Not IndexedVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
If an event is not indexed in a smart contract, it means that the event's parameters are not tagged with the `indexed` keyword. This has implications for how the event data can be searched and filtered when looking through blockchain logs. Without indexing, the event will still emit the data as part of the transaction log, but users won't be able to query for these events using the parameters. They'll have to retrieve the entire set of logs and manually sift through them to find events with the specific data. This can be less efficient and more time-consuming, especially on a blockchain with a high volume of transactions and events.
Open the reportLowInconsistent Grouping Of FunctionsVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The function `isMarketListed()` is not grouped with the other external view functions.
Open the reportLowIndexed Dynamic Data Type In EventVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
When attempting to index dynamic data types like `string`, `bytes`, `array`, or `struct` in Solidity, they don't get stored in their original form. Instead, the Ethereum log system stores the `Keccak-256` hash of these data types. While this approach ensures efficiency and cost-effectiveness, developers must be aware of it to correctly use and interpret logs, and they cannot retrieve the original string from its hash alone.
Open the reportShow 10 more
LowMissing Input ValidationVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The following input validations should be added to prevent unexpected behavior: **RiskStewardReceiver** - When `debounce` is set, it should be checked to be greater than the `UPDATE_EXPIRATION_TIME` to ensure that updates are not processed too quickly. **MarketCapsRiskSteward** - When setting the `maxDeltaBps` it is not checked to be less than some maximum value.
Open the reportLowMissing Zero Address ValidationVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The input `riskSteward` is not checked to be `address(0)`. If it set to be `address(0)`, then it allows a risk parameter configuration to be active while being unsupported, since to check if an update type is supported the following check is used in the codebase ```solidity if (riskParameterConfigs[updateType].riskSteward == address(0)) { revert UnsupportedUpdateType(); } ```
Open the reportLowRepeat CalculationVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
`_validateUpdateStatus()` and `_processUpdate()` both do the same calculation with `_getMarketUpdateTypeKey()`.
Open the reportLowTypos And InconsistenciesVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The following typos and inconsistencies were found in the codebase: **RiskStewardReceiver** - The comment above the error `ConfigNotActive` are not consistent with the error. **MarketCapsRiskSteward** - Often `increase` is used when referring to the max bps, however, it is a limit for increasing and decreasing. - The comment above the error `UpdateNotInRange` uses "our" when it should use "out". - The comment for the parameter `update` in the function `processUpdate()` does not describe the parameter. - The comments above the function `processUpdate()` state that its access is controlled by `AccessControlManager`, when it is only callable by the `RISK_STEWARD_RECEIVER`. The `RiskStewardReceiver` may make calls to this in the functions `processUpdateById()` and `processUpdateByParameterAndMarket()` which can be called by any user. - There is an extra line spacing in the function `_updateSupplyCaps()` that is not consistent with the spacing of the contract.
Open the reportLowUnnecessary ImportsVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The cited imports are not used within their respective contracts and can be removed.
Open the reportLowUnnecessary InheritanceVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The `MarketCapsRiskSteward` inherits `AccessControlledV8` which already inherits `Ownable2StepUpgradeable` and `Initializable`, making it unnecessary to inherit them again. In addition instead of calling `__Ownable2Step_init` and `__AccessControlled_init_unchained()` separately, `__AccessControlled_init()` can be called. Similarly this is the case for `RiskStewardReceiver` as well.
Open the reportLowUsage of Magic NumbersVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The contract contains "magic numbers" (hardcoded numeric values) without any explanation or constants to define their purpose. This reduces code readability and maintainability, making auditing harder and potentially hiding unintended logic or vulnerabilities.
Open the reportLowUser-Defined GettersVenus - RiskOracle Integration · CertiK · 19 Feb 2025Fixed
The linked functions are equivalent to the compiler-generated getter functions for the respective variables.
Open the reportLowInconsistent CommentVenus - PendleOracle Upgrade · CertiK · 26 Dec 2024Fixed
The comment above `_getUnderlyingAmount()` was updated to state that it fetches the amount of underlying or SY token for 1 pendle token. However, this may be misunderstood, because in the case that `RATE_KIND == PT_TO_SY` the `underlyingToken` should be set to the SY token.
Open the reportLowDiscussion On Current Addresses With `DEFAULT_ADMVenus - ACM Commands Aggregator · CertiK · 7 Oct 2024Fixed
The documentation provided state the following: "Normal Timelock contracts on each remote network will have the DEFAULT_ROLE in the AccessControlManager contract. So only Normal VIPʼs on BNB Chain (that will use the Normal Timelocks on the remote networks) will be able to complete the proposed plan, because no other timelock contract will be able to grant and revoke the DEFAULT_ROLE to/from the ACMCommandsAggregator." However, when reviewing the `AccessControlManager` contracts deployed on the remote networks, we noticed that in addition to the normal timelock having the `DEFAULT_ADMIN_ROLE` (if it is deployed), a multi-sign is also given the `DEFAULT_ADMIN_ROLE`. This allows for the `DEFAULT_ADMIN_ROLE` to granted and revoked to/from the ACMCommandsAggregator through a method other than normal VIPs. For example on Ethereum, the `AccessControlManager` contract is at address [0x230058da2D23eb8836EC5DB7037ef7250c56E25E](https://etherscan.io/address/0x230058da2D23eb8836EC5DB7037ef7250c56E25E) and the mutli-sign at address [0x285960C5B22fD66A736C7136967A3eB15e93CC67](https://etherscan.io/address/0x285960C5B22fD66A736C7136967A3eB15e93CC67) has the `DEFAULT_ADMIN_ROLE` along with the normal timelock at address [0xd969E79406c35E80750aAae061D402Aab9325714](https://etherscan.io/address/0xd969E79406c35E80750aAae061D402Aab9325714). Can you please confirm if the intention is to revoke the `DEFAULT_ADMIN_ROLE` from the multi-signs and when this action will take place.
Open the reportShow 10 more
LowMissing Natspec Comment For ContractVenus - ACM Commands Aggregator · CertiK · 7 Oct 2024Fixed
Other files within the codebase include NatSpec comments for the title, author, and summary of the contract. However, `ACMCommandsAggregator` does not have such comments.
Open the reportLowMissing Zero Address ValidationVenus - ACM Commands Aggregator · CertiK · 7 Oct 2024Fixed
In the constructor, the input `_acm` is not checked to ensure it is not the zero address.
Open the reportLowEmitted Event Does Not Specify ReceiverVenus - Risk Fund Upgrade · CertiK · 26 Aug 2024Fixed
The `SweepTokenFromPool` event does not specify the `receiver`. This is inconsistent with the `SweepToken` event which does specify the `to` address.
Open the reportLowTypos And InconsistenciesVenus - Risk Fund Upgrade · CertiK · 26 Aug 2024Fixed
- The comments above the function `sweepTokenFromPool()` use "sweepTokenFromPool" for the documented event as opposed to "SweepTokenFromPool". - The comments above the function `sweepTokenFromPool()` state "ZeroAddressNotAllowed is thrown when tokenAddress/comptroller address is zero". However, it is now also thrown when `receiver` address is zero.
Open the reportLowInconsistent CommentVenus - New Custom Oracle · CertiK · 23 Aug 2024Fixed
The comments above `_getUnderlyingAmount()` states it gets the ETH amount for 1 weETH LRT. However, the function returns the WETH amount. While these values will be the same, we recommend being consistent with the base asset.
Open the reportLow`_max()` InconsistenciesVenus - Two Kinks Interest Rate · CertiK · 31 Jul 2024Fixed
The function `_max()` takes two `int256` as input and returns the larger of the two numbers. However, if they are both negative, then it will cast the greater negative number to a `uint256` causing it to return a large value. This function is only used internally and one of the inputs is always zero, so that these cases are not relevant to the current contract. However, if this function is adapted to be used in the future, then these cases may need to be considered and handled.
Open the reportLowMissing ChecksVenus - Two Kinks Interest Rate · CertiK · 31 Jul 2024Fixed
The `constructor()` does not perform the following checks: - It does not check that `kink1_<kink2_`. If `kink2_` is less than `kink1_` the curve will not have the desired shape. - It does not check that `kink1` is strictly greater than 0. This should be done if the intention is to always have three linear functions. If the intention is to allow two, we recommend considering creating a separate contract to handle this.
Open the reportLowTypos And InconsistenciesVenus - Two Kinks Interest Rate · CertiK · 31 Jul 2024Fixed
The comment at the start of the `TwoKinksInterestRateModel` contracts state " An interest rate model with two different steep increase each after a certain utilization threshold called **kink** is reached." However, the implementation does not just allow for steep increases, in general the implementation allows for the slope to be increased or decreased when each kink is reached. --- The comment above `getBorrowRate()` states that the return value is "scaled by 1e18" when all other comments instead use "scaled by EXP_SCALE" --- The comment above `JUMP_MULTIPLIER_PER_BLOCK` is not consistent with the comments for the `MULTIPLIER_2_PER_BLOCK` or `MULTIPLIER_PER_BLOCK`. It could be reworded to be "The multiplier of utilization rate per block that gives the slope 3 of the interest rate". --- The comments for the inputs `multiplierPerYear_` and `multiplier2PerYear_` in the `constructor()` state "The rate of increase". However, with the current implementation these can be negative values so that it can be the rate of increase or decrease. --- The comment for the input `baseRate2PerYear_` in the `constructor` states "The approximate target base APR after hitting `KINK_1`, as a mantissa (scaled by EXP_SCALE)". However the `baseRate2PerYear_` value represents a discontinuous increase in APR, as the calculation of the rate adds the rate from the previous interval.
Open the reportLowUnchecked Blocks Can Optimize ContractVenus - Two Kinks Interest Rate · CertiK · 31 Jul 2024Fixed
In the function `_getBorrowRate()` the calculations `util - KINK_1` and `util - KINK_2` can be placed in unchecked blocks to save gas. This is because `util` is the output of the call to `utilizationRate` so that it will always be in the interval [0,1e18]. Then `util - KINK_1` is only executed if `util >= KINK_1` so that it cannot underflow. Similarly `util - KINK_2` is only executed if `util >= KINK_2` so that it cannot underflow. If the suggested check is made to ensure that `KINK_1 < KINK_2`, then in addition `KINK_2 - KINK_1` can also be placed in an unchecked block.
Open the reportLow`maxAllowedPriceDifference ` Not InitializedVenus Oracle · Quantstamp · 29 May 2024Fixed
The `maxAllowedPriceDifference` variable is not initialized in the initialize function. It is important to set a default value for this variable to avoid unexpected behavior.
Open the reportShow 10 more
LowMissing Inheritance Reduces ConsistencyVenus Oracle · Quantstamp · 29 May 2024Fixed
Since the `SFrxETHOracle` contract is going to be deployed as the main oracle in the existing `ResilientOracle`, it should inherit from `OracleInterface` to be consistent since the resilient oracle calls the `SFrxETHOracle.getPrice()` function through the mentioned interface.
Open the reportLowMissing Zero Value CheckVenus - sfrxETH oracle adapter · CertiK · 17 May 2024Fixed
While it is unlikely, it is theoretically possible for the returned `priceLow` and `priceHigh` values to be large enough that they exceed the value `EXP_SCALE ** 2`, causing `priceHighInUSD` and `priceLowInUSD` respectively to be 0. While the resilient oracle contract ensures the returned price is nonzero, it is convention within other contracts to make this check sooner within the logic.
Open the reportLowUnprotected InitializerVenus - sfrxETH oracle adapter · CertiK · 17 May 2024Fixed
The contract `SFrxETHOracle` does not protect its `initializer`. An attacker can call the initializer and assume ownership of the logic contract, whereby they can perform privileged operations that trick unsuspecting users into believing that they are the owner of the upgradeable contract.
Open the reportLowIt Is Possible to Have an Invalid Value Inside `trustedRemoteLookup`Venus - Mutichain Governance · Quantstamp · 10 May 2024Fixed
`trustedRemoteLookup` stores the allowed path for sending messages in the format of remote app address + local app address. There is no limitation on the size or structure of this mapping. The value field of the mapping is passed to `LZ_ENDPOINT.send()`, which expects as one of the parameters: remote address concatenated with local address packed into 40 bytes. It is not guaranteed that the value in the `trustedRemoteLookup` mapping is 40 bytes, which could lead to unexpected behavior.
Open the reportLowMissing `_zroPaymentAddress` SpecificationVenus - Mutichain Governance · Quantstamp · 10 May 2024Fixed
The `send()` function in `OmnichainProposalSender` is used to send proposal data to the LayerZero endpoint and includes the `_zroPaymentAddress` parameter. This parameter is required when the executor chooses to pay transaction fees using ZRO tokens instead of native currency. The address provided must meet the following conditions: `require(_zroPaymentAddress == ua || _zroPaymentAddress == tx.origin, "LayerZero: must be paid by sender or origin");` This requirement ensures that the transaction fees are paid either by the user application (ua, typically `OmnichainProposalSender`) or the transaction's origin (`tx.origin`).
Open the reportLowPossibility of Accepting Messages From Compromised RemotesVenus - Mutichain Governance · Quantstamp · 10 May 2024Fixed
The `OmnichainGovernanceExecutor` contract is set up as a non-blocking app, which allows for the replaying of unsuccessful messages via `retryMessage()` which makes a subsequent call to `_nonblockingLzReceive()` that attempts to retry the failed message. This can be an issue if a trusted remote address is compromised and is removed as a trusted remote since neither of the above mentioned functions perform any validation checks on whether the address behind a `_srcChainId` is still a `trustedRemote`.
Open the reportLow`ensureNotPaused()` Is Not Used ConsistentlyVenus - VAI Controller Upgrade · CertiK · 26 Apr 2024Fixed
The function `liquidateVAI()` makes the following check: ```solidity require(!comptroller.protocolPaused(), "protocol is paused"); ``` as opposed to using `_ensureNotPaused()`.
Open the reportLowBreaking Change in Error Handling of Some FunctioVenus - VAI Controller Upgrade · CertiK · 26 Apr 2024Fixed
Functions `mintVAI()` and `repayVAIFresh()` change how certain errors are handled, moving from returning an error to reverting. Protocols may be designed to interact and handle errors in a specific manner, causing issues with compatibility.
Open the reportLowMissing Zero Address CheckVenus - VAI Controller Upgrade · CertiK · 26 Apr 2024Fixed
Function `repayVAIBehalf()` does not ensure that the input address `borrower` is not `address(0)`.
Open the reportLowReturn Value Of `repayVAI()` Is ChangedVenus - VAI Controller Upgrade · CertiK · 26 Apr 2024Fixed
The function, `repayVAI()`, returns `repayVAIFresh()` which has been changed to return the `repaidAmount` as opposed to the `burn` value. While this change was implemented to fix an error in the logic of `liquidateVAIFresh()`, it changes the return value of an external-facing function. Consequently, a protocol that utilizes the return value of this function may have their logic broken by this change.
Open the reportShow 10 more
LowSuggested Changes to Natspec CommentsVenus - VAI Controller Upgrade · CertiK · 26 Apr 2024Fixed
- The comments above functions `repayVAI()` and `repayVAIBehalf()` contain the following statement: "The repay function transfers VAI into the protocol and burn, reducing the borrower's borrow balance." The statement may read better as "The repay function transfers VAI interest into the protocol and burns the rest, reducing the borrower's borrow balance." - The following comment above function `repayVAI()` is also relevant to `repayVAIBehalf()`: "Before repaying an asset, users must first approve the VAI to access their VAI balance."
Open the reportLowPotential Implicit WhitelistingVenus - Mesh Architecture · CertiK · 19 Apr 2024Fixed
In order for an address to be whitelisted, they must be whitelisted on both the sending and receiving chain. With the mesh-architecture it allows for an address to be implicitly whitelisted between two chains. This may cause an issue if an address should not be whitelisted when transferring directly to/from a specific chain, but it is necessary to whitelist a path between the chains. Note that this is a strictly hypothetical scenario and we are not aware of any protocols where such a scenario would cause an issues.
Open the reportLowSame Send And Receive Limits May Cause UnexpectedVenus - Mesh Architecture · CertiK · 19 Apr 2024Fixed
Both contracts set the single send and receive limit to `10000`. Similarly, they set the max daily send and receive limits to be `50000`, where this value is determined by the USD value when the function is called. As such tokens may be sent when the value of `XVS` dropped dramatically, so that when the message is to be received and the price recovers it will exceed the single receive limit or the max daily receive limit. Alternatively, the price of XVS may drop on one chains oracle, but remain constant on another chains oracle.
Open the reportLow`OneJumpOracle` Must Return a Value Scaled to the Underlying Asset DecimalsVenus - Correlated Oracles · Quantstamp · 10 Apr 2024Fixed
`OneJumpOracle` uses `INTERMEDIATE_ORACLE` oracles that follow the Compound oracle convention, which means that they return values that are scaled to `36 - correlatedDecimals`. This is incompatible with the requirement from the `CorrelatedTokenOracle`, where it expects the `OneJumpOracle` to return a value scaled to the underlying asset decimals.
Open the reportLowOutdated Solidity VersionVenus - Correlated Oracles · Quantstamp · 10 Apr 2024Fixed
The project uses Solidity version `0.8.13`, which is not recommended for deployment.
Open the reportLowTypos And InconsistenciesVenus - redeemAndUnwrap · CertiK · 19 Mar 2024Fixed
### INativeTokenGateway - The comment above sweep token states “@dev Emitted when wNativeToken is swept from the contract”. However, it sweeps the input token not necessarily wNativeToken. ### NativeTokenGateway - The parameter `redeemTokens` in function `_redeemAndUnwrap()` does not reflect the fact that this value can represent either `redeemTokens` from `redeemAndUnwrap()` or `redeemAmount` from `redeemUnderlyingAndUnwrap()`.
Open the reportLowMismatch Between Function Name and String for Access ControlTime-Based Contracts (Venus) · Quantstamp · 18 Mar 2024Fixed
In the `setLastRewardingBlocks()` function of the `RewardsDistributor` contract, the input string to `_checkAccessAllowed()` is `setLastRewardingBlock` instead of `setLastRewardingBlocks` as the function name.
Open the reportLowMissing Check for the Configured Mode of Reward Distributors in `PoolLens`Time-Based Contracts (Venus) · Quantstamp · 18 Mar 2024Fixed
In the `_calculateNotDistributedAwards()` function of the `PoolLens` contract, the rewards are calculated based on the mode (time-based or block-based) that the reward distributor is configured. Since all the reward distributor contracts deployed on the same chain should use the same mode as `PoolLens`, consider adding a check to ensure they are in the same mode to reduce the risk of misconfiguration during contract deployment. Also, the local variable, `isTimeBased`, shadows a state variable inherited from the `TimeManagerV8`. It is generally recommended to avoid shadow variables as a best practice.
Open the reportLowMissing Input ValidationTime-Based Contracts (Venus) · Quantstamp · 18 Mar 2024Fixed
It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. The following is a non-exhaustive list of missing input validations: 1. In the `SetterFacet` contract, the `_setXVSVToken()` function sets the VToken contract corresponding to the XVS token. As a best practice, there can be a check on the provided argument `xvsVToken_` to ensure that the underlying token of `xvsVToken_` matches the configured `xvs`. 2. The constructor of the VToken contract should validate that the value of `MAX_BORROW_RATE_MANTISSA` is not greater than `1e18`. 3. In the `constructor()` of the `SequencerChainlinkOracle` contract, check that sequencer is not set to the zero address.
Open the reportLowEvent Emission for Unchanged Delegate StatusVenus Protocol: Native Token Gateway · Quantstamp · 5 Mar 2024Fixed
The `updateDelegate` function emits a `DelegateUpdated` event for every call, even if the delegate's approval status remains unchanged.
Open the reportShow 10 more
Low`delete` Keyword Can Be UsedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
In the functions `cancelTransaction()` and `executeTransaction()` the keyword `delete` can be used in place of setting `queuedTransactions[txHash]` to false.
Open the reportLow`pId` Can Be Used Instead of `newProposal.id`Venus - Multichain Governance · CertiK · 26 Feb 2024Fixed
In function `_nonblockingLzReceive()`, the `newProposal.id` value is used as a key in the `proposals` mapping. The `newProposal.id` is the `pId`; the mapping can be set using the `pId` as the key instead.
Open the reportLowCase Not Handled By `fallbackWithdraw()`Venus - Multichain Governance · CertiK · 26 Feb 2024Fixed
Function `execute()` allows a message to be executed with a `msg.value` of 0, which may lead to a failed message. Such a message may need to be removed, but cannot be removed via function `fallbackWithdraw()`, due to the requirement that `originalValue_` input is nonzero. As a result, a failed message of this kind may become stuck in `OmnichainProposalSender`.
Open the reportLowCheck-Effect-Interaction Pattern ViolatedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
While the `nonreentrant` modifier and access restriction can prevent reentrancy, we recommend following check-effect-interaction wherever possible as added protection and best practice.
Open the reportLowEligibility May Be Expired at Time `retryExecute(Venus - Multichain Governance · CertiK · 26 Feb 2024Fixed
Function `_isEligibleToSend()` is used as a check in function `execute()`. However, if the proposal is not properly executed, it gets stored, and the execution can be retried at a later time. The checks made in `_isEligibleToSend()` are based on a 24-hour window. If the second attempts are made outside the 24 hour window in which the execution was initially tried, then it is possible that the number of proposals exceeds the limit during the retry. Additionally, note that when proposal messages are retried, it may result in two proposals sent during the same `block.timestamp`, a scenario that is supposed to be prevented by function `_isEligibleToSend()`.
Open the reportLowEvent Should Include Chain IdVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The event `SetMaxDailyLimit` should include the chain id to make it easier to track which chain's limit is being updated.
Open the reportLowHardcoded Boolean Can Be UsedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
In the function `upsertSignature()` there is the following logic: ```solidity if (active_[i] && signature.length == 0) { functionRegistry[sigHash] = signatures_[i]; emit FunctionRegistryChanged(signatures_[i], active_[i]); } ``` However, if this if-block is executed then necessarily `active_[i]` is true so that the emitted event can use `true` as opposed to reading the value. Similarly in the following logic: ```solidity else if (!active_[i] && signature.length != 0) { delete functionRegistry[sigHash]; emit FunctionRegistryChanged(signatures_[i], active_[i]); } ``` If this if-block is executed then necessarily `active_[i]` is false so that the emitted event can use `false` as opposed to reading the value.
Open the reportLowInaccurate PayloadVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The payload that is sent or stored at `storedExecutionHashes[_pId]` in the `execute()` function includes a second layer of encoding the pId with the initial `payload_`. The parameter comment in functions `retryExecute()` and `fallbackWithdraw()` here makes it seem as if the original `payload_` input in function `execute()` should be used again; this will result in a revert. The comments could either be rewritten to accurately specify the extra layer of encoding, or else this encoding step could be done within the logic to ensure the check will come out correctly when comparing the hashes. The parameter `payload_` in the comments above `estimateFees()`, `retryExecute()` and `fallbackWithdraw()` are also missing the inclusion of the `proposalType` at the end of the encoding.
Open the reportLowIncrement OptimizationsVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
In general, the counter in a for loop can be incremented or decremented in an unchecked block as it cannot overflow or underflow, saving gas as it will not perform a check for overflow or underflow. Additionally, it saves a small amount of gas to increment an index in a `for` loop from the left instead of from the right side as it performs fewer operations. ### OmnichainGovernanceExecutor.sol - In function `addTimelocks()`, incrementing variable `i` can be incremented in an `unchecked` block. - In function `execute()`, incrementing variable `i` can be incremented in an `unchecked` block. - In function `cancel()`, incrementing variable `i` can be - incremented in an `unchecked` block; - incremented from the left; - implicitly initialized at 0, it does not need to be set to 0 explicitly; - In function `_queue()`, incrementing variable `i` can be incremented in an `unchecked` block. ### OmnichainExecutorOwner.sol - In function `upsertSignature()`, incrementing variable `i` can be - incremented in an `unchecked` block; - incremented from the left;
Open the reportLowInefficient Memory ParameterVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
One or more parameters with `memory` data location are never modified in their functions and those functions are never called internally within the contract. Thus, their data location can be changed to `calldata` to avoid the gas consumption copying from calldata to memory.
Open the reportShow 10 more
LowInterface Defined In Same File It Is UsedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The interface `IOmnichainGovernanceExecutor` in `OmnichainExecutorOwner` can be defined in a separate file so that it is easier for future projects to import them and avoid duplication.
Open the reportLowIssues With `ValidChainIds`Venus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The mapping `validChainIds` is used to determine if an input remote chain id is valid. However, the address of the contract on the remote chain to receive messages sent by this contract should only be set if the remote chain is valid. Only `setTrustedRemoteAddress()` is implemented and there is no method such as `setTrustedRemote()` that can set set it to the default empty path. Thus once set `trustedRemoteLookup[remoteChainId_].length` will always be nonzero. `validChainIds` can then invalidate a remote chain in the event that the remote address was set and it needs to be removed as a valid chain. Alternatively, `validChainIds` can be removed and instead the `setTrustedRemote()` method (see [here](https://github.com/LayerZero-Labs/solidity-examples/blob/main/contracts/lzApp/LzApp.sol)) can be implemented so that in the event that a remote chain should be invalidated `trustedRemoteLookup[remoteChainId_]` can be set to the empty path and the check ```solidity require(trustedRemote.length != 0, "OmnichainProposalSender: destination chain is not a trusted source"); ``` would always ensure that the remote chain is valid. --- Function `retryExecute()` should include a relevant check based on the above to ensure the input `remoteChainId_` is still allowed at the time of the second attempt.
Open the reportLowMissing or Incomplete NatSpecVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
### BaseOmnichainControllerDest - There are no comments above the function `_isEligibleToReceive()`. ### BaseOmnichainControllerSrc - There are no comments above the function `_isEligibleToSend()`. - There are no comments above the function `_ensureAllowed()`. ### OmnichainProposalSender - In function `execute()` there is a missing natspec comment specifying the access restriction of the function ### OmnichainGovernanceExecutor - There are no comments above the function `_blockingLzReceive()`. - There are no comments above the function `_nonblockingLzReceive()`. - There are no comments above the function `_queue()`. - There are no comments above the function `_queueOrRevertInternal()`. ### OmnichainExecutorOwner - The comments above the `fallback()` function do not mention the access restriction, input, or return value. ### ITimelock - The comments above `executeTransaction()` do not include a description of the return value. ### TimelockV8 - The comments above `setDelay()` do not mention the emitted event or access restriction. - There are no comments above the functions `GRACE_PERIOD()`, MINIMUM_DELAY()`, `MAXIMUM_DELAY()`, or `getBlockTimestamp()`. - The comments above `acceptAdmin()` do not mention the emitted event or access restriction. - The comments above `setPendingAdmin()` do not mention the emitted event or access restriction. - The comments above `queueTransaction()` do not mention the emitted event or access restriction. - The comments above `cancelTransaction()` do not mention the emitted event or access restriction. - The comments above `executeTransaction()` do not mention the emitted event, access restriction, or return value. - The comments above `queueTransaction()` do not mention the emitted event or access restriction.
Open the reportLowMissing Proposal Status Checks and UpdatesVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
While it is understood that the in-scope `TimelockV8` contract and `OmnichainProposalSender` work together with the `OmnichainGovernanceExecutor` contract to ensure the proper intended behavior in proposal queuing, execution, and cancellation, the following checks should be added to the `OmnichainGovernanceExecutor` contract in order to ensure correct functioning in the event the timelock contracts are updated through `addTimelocks()`. For instance, if the timelocks are ever updated, in the current `OmnichainGovernanceExecutor` contract, the fact that previously executed `proposals` remained set to `true` in `queued` may potentially allow for the possibility of replaying the execution of a proposal, since there are no checks within the `OmnichainGovernanceExecutor` contract to ensure the proposal has not already been executed or cancelled. Additionally, currently there should be only one source chain providing proposals, so it is assumed that all incoming `pId` values have not previously been set. However, the `OmnichainGovernanceExecutor` contract has a mapping to keep track of limits for multiple source chains (`chainIdToMaxDailyReceiveLimit`). In the event that multiple source chains send proposals, the `pId` method may no longer be unique. Checks are suggested below in order to ensure the `OmnichainGovernanceExecutor` and `TimelockV8` contracts can function properly, independent of one another. ### OmnichainGovernanceExecutor.sol - Function `execute()` is missing a check ensuring that `proposal.executed` is previously false before updating it to true. - Function `execute()` is missing a check ensuring that `proposal.cancelled` is false before executing it. - Function `execute()` should update `queued[proposalId_]` to false as part of the logic. - Function `cancel()` is missing a check ensuring that `proposal.cancelled` is false before executing it. - Function `cancel()` should update `queued[proposalId_]` to false as part of the logic. - Function `_nonblockingLzReceive()` is missing a check ensuring that `proposals[pId]` is not already set before updating it. - Function `_queue()` is missing a check ensuring that `queued[proposalId_]` is false before setting it to true. ### TimelockV8.sol - Function `queueTransaction()` is missing a check ensuring that `queuedTransactions[txHash]` is false before updating it to true. - Function `cancelTransaction()` is missing a check ensuring that `queuedTransactions[txHash]` is true before updating it to false.
Open the reportLowPotential Issues With `msg.sender` Being RefundedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
Function `retryExecute()` can be called by anyone, because there is a check ensuring the payload entered was one previously attempted at execution, but which failed. In `retryExecute()`, the refund is directed to go to the `msg.sender` currently making the call, even though the account that initially providing the `msg.value` was the account that called function `execute()`. The account attempting `retryExecute()` may not be the original account that called `execute()`, since this function is free to be called by anyone. It is possible that, in the original call to `execute()`, a large `msg.value` was included, with the idea being any difference would be refunded; it is possible that the execution fails for some other reason besides insufficient fees, however. In such a case, someone may watch for such a failure and attempt to call `retryExecute()` with the opportunity to seize the refund, which may be significant. Additionally, a low-level call is made to the refund recipient when sending the refund, and this is done before all logic has been completed by the endpoint. This may pose a system-reentrancy risk.
Open the reportLowPrevious State Variable Value Can Be Included inVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
In the events cited, it may be beneficial for on-chain reference to include the previous value of the updated state variable.
Open the reportLowSpecific Imports Not Consistently UsedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
Many of the added files use specific imports, however, some import the entire file.
Open the reportLowTypos and InconsistenciesVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
### BaseOmnichainControllerSrc.sol - The comments above the contract `BaseOmnichainControllerSrc` have an additional "*" before the security-contact. - In the comments above function `setMaxDailyLimit()`, parameter `limit_` is not representing an amount in USD. - In the comments above function `setMaxDailyLimit()`, there is stray notation ")." included at the end of line `@param limit_`. - The comment in function `_isEligibleToSend()` states multiple commands cannot be sent for a `dstChainId_` within a proposal, but this is allowed; it is that multiple proposals cannot be sent within the same `block.timestamp`. ### BaseOmnichainControllerDest.sol - In the comments above function `setMaxDailyReceiveLimit()`, parameter `limit_` is not representing an amount in USD. - in the comments above function `setMaxDailyReceiveLimit()` the parameter `@param chainId_` says it is representing the destination chain ID, but it is actually representing source chain id. ### ITimelock.sol - In each of the comments cited above, the word "signature" is misspelled as "Ssignature."
Open the reportLowUnnecessary `Payable` Casting May Lead to LockedVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
Function `fallbackWithdraw()` is a privileged function used to remove failed proposals that cannot be executed. The function includes a `payable` casting, even though it must be used to remove proposals and remove the corresponding amount of funds from the contract. If the `_owner` includes a `msg.value` with the call, then this amount could become stuck in the contract, since it does not correspond to a proposal.
Open the reportLowUnnecessary `uint8` CastingVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The `proposal.proposalType` is already `uint8` type, it is not necessary to cast it explicitly.
Open the reportShow 10 more
LowUnnecessary CheckVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
Function `fallbackWithdraw()` includes the following check: ```solidity require(address(this).balance >= originalValue_, "OmnichainProposalSender: insufficient native balance"); ``` This check is unnecessary because it is checked that `keccak256(execution) == hash`, meaning that the `originalValue_` included as part of the `hash` construction was sent to the contract but currently has not successfully been transferred out.
Open the reportLowUnprotected InitializerVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
The logic contract `OmnichainExecutorOwner` does not protect its initializer function. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract.
Open the reportLowUse Temporary Variable To Save Reading From StoraVenus - Multichain Governance · CertiK · 26 Feb 2024Fixed
### OmnichainGovernanceExecutor - In the function `execute()`, `proposalTimelocks[uint8(proposal.proposalType)]`, `proposal.targets.length`, and `proposal.eta` can be stored in a temporary variable to avoid being repeatedly read from storage. - In the function `_queue`, `proposal.proposalType` can be stored in a temporary variable to avoid being repeatedly read from storage. - In the function `cancel()`, `proposalTimelocks[proposal.proposalType]`, `proposal.targets.length`, and `proposal.eta` can be stored in a temporary variable to avoid being repeatedly read from storage. - In the function `_queue()`, `proposal.targets.length` and `proposal.proposalType` can be stored in a temporary variable to avoid being repeatedly read from storage.
Open the reportLowSpecific Imports Not Consistently UsedVenus - Oracle for wstETH on Ethereum · CertiK · 26 Jan 2024Fixed
Many of the added files use specific imports, however, some import the entire file.
Open the reportLowTypos And InconsistenciesVenus - Oracle for wstETH on Ethereum · CertiK · 26 Jan 2024Fixed
In the function `getPrice()`, "wstETH" is misspelled as "wsETH".
Open the reportLow`for` Loop OptimizationVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
In general, the counter in a for loop can be incremented or decremented in an unchecked block as it cannot overflow or underflow, saving gas as it will not perform a check for overflow or underflow. Additionally, it saves a small amount of gas to increment an index in a `for` loop from the left instead of from the right side as it performs fewer operations.
Open the reportLow`minterToCap` Must be Updated Consistently with AVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
- An account given a nonzero `minterToCap` value is assumed to be given access to functions `mint()` and `burn()`. - An account given access to functions `mint()` or `burn()` is assumed to be given a nonzero `minterToCap` value. - An account given access to the `burn()` function is assumed to be given access to mint, and vice versa. While these privileges are dependent on one another, each part of the privilege is given independently, increasing the chance for inconsistent update.
Open the reportLowCases Not Explicitly HandledVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
If `active_[i] && signature.length != 0` or `!active_[i] && signature.length == 0` then none of the if-blocks will be executed. If either of these combinations are input by accident, then it can cause confusion as there is nothing to indicate this other than a lack of event emitted.
Open the reportLowCheck-Effect-Interaction Pattern ViolatedVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
An external call to transfer the `innerToken` is made first, and then the `outboundAmount` is updated and checked. While it is assumed that the `innerToken` is the team's `XVS` token, following the check-effect-interaction pattern can reduce any unforeseen risk.
Open the reportLowChecking the `oracle_` Input is Nonzero Can Be DoVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
In `BaseXVSProxyOFT.sol`, the check that the `oracle_` address input on deployment is nonzero is made after setting state variables `ld2sdRate` and `innerToken`. The check with `ensureNonzeroAddress` can be made at the beginning of the constructor with the other zero address checks.
Open the reportShow 10 more
LowConsider Reverting As Opposed To Having Empty ImpVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
The function `renounceOwnership()` is overriden to have an empty implementation to prevent ownership from accidentally being revoked. We recommend considering having the function revert with a descriptive error message indicating this function has been disabled for the contract. This will help avoid any potential confusion in the future if someone was to call this function.
Open the reportLowDiscrepancy with Blacklisting in XVSVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
In the `XVS` contract, a blacklisted account cannot be minted tokens. However, on the chain in which the account is blacklisted, anyone can still transfer tokens to the blacklisted account, transfer tokens from the blacklisted account, the account can be approved to move others' tokens, and the account can have their tokens burned. As a consequence, the blacklisted account can easily acquire tokens, and they can still bridge these tokens to another chain where they may not be blacklisted. Moreover, in `XVSProxyOFTSrc`, there is no check from this side of whether the account bridging tokens is blacklisted on other chains, and there is no way to blacklist the user from the source chain. As a result, a user can transfer in their tokens into the `XVSProxyOFTSrc` contract, but if they are blacklisted on the destination chain, there is no equivalent amount given out. In this way, these tokens become stuck within the protocol and cannot be removed without intervention via centralized accounts.
Open the reportLowDiscussion On `XVSProxyOFTDest` vs. `XVSProxyOFTSVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
There are currently two bridge contracts `XVSProxyOFTSrc` and `XVSProxyOFTDest`. The main difference being that `XVSProxyOFTSrc` locks tokens if transferring to another chain and gives locked tokens when receiving tokens from another chain, while `XVSProxyOFTDest` burns or mints tokens when transffering to another chain or receiving from another chain respectively. As such we assume that the design is to deploy a single `XVSProxyOFTSrc` on BSC as it holds the current supply of XVS and all other chains should only receive XVS that has been bridged out of BSC. Furthermore we assume that all other chains will have a single `XVSProxyOFTDest` contract deployed to bridge XVS tokens. Last, it is assumed that only one address (the bridge contract) will utilize a nonzero `minterToCap` value per chain, except in extenuating circumstances in which funds become stuck in the protocol based upon the restrictions of the design (such as the bridged amount exceeding the mint cap of the bridge for the chain). Furthermore, it is assumed that any use of this minting capability beyond the bridge associated with the chain will be carefully assessed in order to maintain the invariant that `totalSupply()` of XVS on BSC is equivalent to the `circulatingSupply()` of the BSC bridge contract, plus the amount `outboundAmount`. A similar assumption is made for the `fallbackWithdraw()` function in `XVSProxyOFTSrc`. Please let us know if these assumptions are correct and provide further information if they do not align with the design intent.
Open the reportLowDiscussion On DesignVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
It is our understanding that the design of these contracts are for `XVS`, however, they also include functionality to check balances and potentially handle deflationary tokens. We would like to ensure these contracts are only to be used for `XVS` and that no other deflationary or token that implements hooks should be considered.
Open the reportLowDuplicate Zero Address ChecksVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
The function `initialize()` in the contract `XVSBridgeAdmin` checks that the input `accessControlManager_` is not the zero address via `ensureNonzeroAddress`. However, it also calls `__AccessControlled_init()`, which will call `_setAccessControlManager()` that has the following check ```solidity require(address(accessControlManager_) != address(0), "invalid acess control manager address"); ``` Thus it will check that the input `accessControlManager_` is not the `address(0)` twice. --- The function `transferBridgeOwnership()` in the contract `XVSBridgeAdmin` checks that the input `newOwner_` is not the zero address via `ensureNonzeroAddress`. However, it calls `XVSBridge.transferOwnership()`, which calls `transferOwnership()` in OpenZeppelin's `Ownable` contract that also checks the input is not the zero address.
Open the reportLowInconsistency With `cap` CheckVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
In the function `fallbackDeposit()` it is checked that the new `outboundAmount` after the deposit will not exceed the `_sd2ld(type(uint64).max)`. As some chains like Aptos use `uint64` to represent balances, the maximum balance on their chain would be `type(uint64).max` and thus `_sd2ld(type(uint64).max)` represents this maximum amount using the local decimals. However, this check is not performed in `_debitFrom()`, so that it is possible to exceed this cap using normal chain operations and then preventing `fallbackDeposit()` from being called.
Open the reportLowMissing Input ValidationVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
### TokenController.sol - In function `_increaseMintLimit()` there is a missing check that the input `amount_` is no larger than the `minterToMintedAmount` for the input `from_` address. This may cause an ambiguous error message due to underflow in the calculation of `totalMintedNew` in the function. - In function `migrateMinterTokens()`, there is no check that the `source_` and `destination_` addresses are not the same. If they are the same address, the update to mapping `minterToMintedAmount` for that address will not accurately reflect the minted amount (e.g. an address with `mintedToMinted` amount of 100 originally will double to 200 as long as the `destinationCap` is not exceeded). ### XVSProxyOFTSrc.sol - In the function `fallbackWithdraw()` there is no check that `amount_` is no larger than `outboundAmount`. In this case, an ambiguous error message due to underflow will be thrown. - In the function `_creditTo()` there is no check that `amount_` is no larger than `outboundAmount`. In this case, an ambiguous error message due to underflow will be thrown.
Open the reportLowMissing or Incomplete NatSpecVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
### XVSProxyOFTSrc - The comments above `fallbackWithdraw()` do not include the emmited event or the access restriction. - The comments above `dropFailedMessage()` do not include the emmited event or the access restriction. - The comments above `circulatingSupply()` do not include the return value. - There are no comments above the function `_debitFrom()`. - There are no comments above the function `_creditTo()`. ### XVSProxyOFTDest - The comments above `dropFailedMessage()` do not include the emmited event or the access restriction. - The comments above `circulatingSupply()` do not include the return value. - There are no comments above the function `_debitFrom()`. - There are no comments above the function `_creditTo()`. ### XVSBridgeAdmin - The comments above `fallback()` do not include the return value. - The comments above `setTrustedRemoteAddress()` do not include the access restriction or a summary of the functionality. - The comments above `upsertSignature()` do not include the access restriction or emitted events. - The comments above `transferBridgeOwnership()` do not include the potential error. - The comments above `isTrustedRemote()` do not include the return value. - The comments above `_getFunctionName()` do not include the input parameter and return value. - There are no comments above the function `bytesToAddress()`. ### BaseXVSProxyOFT - The comments above the `constructor()` do not include the emitted events. - The comments above `removeTrustedRemote()` do not include the access restriction or emitted event. - The comments above `token()` do not include the return value. - There are no comments above the function `_isEligibleToSend()`. - There are no comments above the function `_isEligibleToReceive()`. - There are no comments above the function `_transferFrom()`. - There are no comments above the function `_nonblockingLzReceive()`. - There are no comments above the function `_ld2sdRate()`. ### TokenController - The comments above the function `_isEligibleToMint()` do not include the emitted event or potential error. - The comments above the function `_increaseMintLimit()` do not include the emitted event. - The comments above the function `_ensureAllowed()` do not include the input parameter or the potential error.
Open the reportLowPausing Behavior in `XVS`Venus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
`mint()` and `burn()` can be paused, however users can still transfer tokens when the contract is paused. We would like to ensure that this is the intended behavior.
Open the reportLowRedundant Getter FunctionsVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
Function `isBlacklisted()` is a view function for mapping `_blacklist`, however, this mapping is public.
Open the reportShow 10 more
LowRepeated Check If Transaction Is SuccesfulVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
The function `lzReceive()` in [https://github.com/LayerZero-Labs/solidity-examples/blob/main/contracts/lzApp/LzApp.sol](https://github.com/LayerZero-Labs/solidity-examples/blob/main/contracts/lzApp/LzApp.sol) makes the check ```solidity require( _srcAddress.length == trustedRemote.length && trustedRemote.length > 0 && keccak256(_srcAddress) == keccak256(trustedRemote), "LzApp: invalid source sending contract" ); ``` If this succeeds then it calls `_blockingLzReceive()`, which is overriden in [https://github.com/LayerZero-Labs/solidity-examples/blob/main/contracts/lzApp/NonblockingLzApp.sol](https://github.com/LayerZero-Labs/solidity-examples/blob/main/contracts/lzApp/NonblockingLzApp.sol) to call `nonblockingLzReceive()`, which then calls `_nonblockingLzReceive()`. However, this contract overrides this to then perform the same check again: ```solidity require( _srcAddress.length == trustedRemote.length && trustedRemote.length > 0 && keccak256(_srcAddress) == keccak256(trustedRemote), "LzApp: invalid source sending contract" ); ``` While this will ensure if there are any updates to trusted remotes between when a message fails and is retried, a check can be added when retrying the message to prevent making duplicate checks on successful messages.
Open the reportLowTokens Become Locked in the Contract When `toAddrVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
In the case where XVS tokens are bridged to the `XVSProxyOFTSrc` contract itself, the `_creditTo()` logic will subtract the `amount_` from the `outboundAmount` while keeping the corresponding `XVS` in the contract, in order to account for the fact that it is no longer bridged to one of the destination chains, and is now technically part of the `circulatingSupply()` on the source chain. However, the only way to remove tokens bridged to the `XVSProxyOFTSrc` contract itself would be to use the `fallbackWithdraw()` function, in which case, the `amount_` would be subtracted from the `outboundAmount` a second time, causing a discrepancy in the accounting of the variable. Consequently, any funds a user accidentally sends directly to the bridge rather than sending to their own account will cause the tokens to be permanently locked in the bridge, or else will cause an issue with `outboundAmount` where eventually someone else's tokens will be permanently locked in the bridge. Additionally, if anyone directly transfers tokens to the bridge, they will not be accounted for in the `outboundAmount`, so they also cannot be removed with `fallbackWithdraw()` without causing an issue with this variable.
Open the reportLowTypos And InconsistenciesVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
### XVSProxyOFTSrc - The comment above the variable `outboundAmount` states "total amount is transferred from this chain to other chains." which should be "Total amount that is transferred from this chain to other chains." - In the comment above the event `DropFailedMessage`, the word "successful" is misspelled as "successfull." ### XVSProxyOFTDest - In the comment above the event `DropFailedMessage`, the word "successful" is misspelled as "successfull." ### XVSBridgeAdmin - The comment above the function `upsertSignature()` does not describe that this is a setter function for the registry. - In the comment above function `transferBridgeOwnership()`, "transfer" should be "transfers". ### BaseXVSProxyOFT - In the comment above the `whitelist` mapping, the word "applicable" is misspelled as "appicable." - In the comment above event `SetWhitelist`, the word "emitted" is misspelled as "emmited" - In the comment above function `setWhitelist()`, the word "address" is misspelled as "Adress" - In the comment above the `constructor()`, "No" should either be "No." or "Number". ### TokenController.sol - In the comment above the `minterToMintedAmount` mapping, the word "minted" is misspelled as "m inted." - In the comment above functions `setMintCap()` and `_increaseMintLimit()` the word "minting" is misspelled as "miniting." ### XVS.sol - In the comment above `mint()` function the description "@param account_ Address to which tokens be assigned" reads correctly as "account_ Address to which tokens are assigned." - In the comment above `mint()`, there is an incorrect description of error `MintNotAllowed`. The error should say it is thrown when minting is not allowed to `to_` address; instead it says the error is thrown when minting is not allowed to the `from_` address. - In the comment above `mint()`, the error description "MintLimitExceed is thrown when minting amount exceed the maximum cap" reads correctly as "MintLimitExceed is thrown when minting amount exceeds the maximum cap"
Open the reportLowUnchecked Blocks Can Optimize ContractVenus - XVS Token Bridge · CertiK · 26 Dec 2023Fixed
In the function `_isEligibleToMint()` the logic: ```solidity if (totalMintedNew > mintingCap) { revert MintLimitExceed(); } ``` ensures that `mintingCap >= totalMintedNew`. Thus it is impossible for `mintingCap - totalMintedNew` to underflow so it can be placed in an unchecked block.
Open the reportLowCritical Role Transfer Not Following Two-Step PatternVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The owner of the contracts can call `transferOwnership()` to transfer the ownership to a new address. If an uncontrollable address is accidentally provided as the new owner address then the contract will no longer have an active owner, and functions with the `onlyOwner` modifier can no longer be executed.
Open the reportLowExtensive Reliance on External ApplicationsVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The protocol depends on LayerZero for bridging XVS tokens to other chains. As such, the token on each network can be impacted by the possible vulnerabilities in the bridging mechanism. This is an inherent risk of integrating with network bridges, not a specific vulnerability of the token implementation.
Open the reportLowInconsistent Data Format For Trusted Remotes in The XVS BridgeVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The owner of the bridge can set and update trusted remote addresses to verify that interactions with LayerZero are generated from trusted contracts on remote chains. Trusted remote addresses are stored in the mapping `trustedRemoteLookup` where the key is the remote chain ID and the data stored is bytes in the format of `abi.encode(<REMOTE_ADDRESS>, <LOCAL_ADDRESS>)`. Setting trusted remote addresses is performed by calling `setTrustedRemoteAddress()` where the remote address is passed as an argument and then encoded in the aforementioned format. Similarly, `setTrustedRemote()` allows for setting a trusted remote with data encoded off-chain. Since `setTrustedRemote()` does not validate the input data, it is entirely possible to encode the incorrect data or encode it in the incorrect format. If the `trustedRemoteLookup` mapping does not contain data in the correct format, bridging tokens to another chain will fail.
Open the reportLowInsufficient Validation in `isTrustedRemote()`Venus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The function `isTrustedRemote()` will return true if `_srcAddress` is an empty array and if the `_srcChainId` has not been added to `trustedRemoteLookup` yet.
Open the reportLowMissing Input ValidationVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
* `VTreasuryV8.withdrawTreasuryToken()`: - Validate that `tokenAddress` and `withdrawAddress` are not `address(0)` to avoid losing funds. - Validate that `withdrawAmount` is not 0. * `VTreasuryV8.withdrawTreasuryNative()`: - Validate that `withdrawAddress` is not `address(0)` to avoid loss of funds. - Validate that `withdrawAmount` is not 0. * `BaseXVSProxyOFT.setMaxSingleTransactionLimit()`: * Validate that the `limit_` is less than or equal to the `chainIdToMaxDailyLimit`. * `BaseXVSProxyOFT.setMaxSingleReceiveTransactionLimit()`: * Validate that the `limit_` is less than or equal to the `chainIdToMaxDailyReceiveLimit`. * `ResilientOracle.constructor()`: * Validate that `nativeMarketAddress` is not the zero address.
Open the reportLowPossible Failures of Bridging XVS TokensVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
This issue highlights the potential failures users may encounter when bridging the XVS tokens. The following scenario may prevent users from sending XVS tokens from the source chain: 1. The oracle temporarily fails due to reasons including being paused by the owner, incorrect address configuration, or price validation failures. 2. The transfer amount exceeds the single or daily sending transaction limit. 3. The transfer amount is too small, becoming zero after removing dust. 4. The sender is blacklisted by the XVS token. 5. The destination bridge is not configured as a trusted remote. The following scenario may prevent users from receiving XVS tokens on the destination chain. Users may retry the failed messages after some period: 1. The oracle temporarily fails due to reasons including being paused by the owner, incorrect address configuration, or price validation failures. 2. The transfer amount exceeds the single or daily receiving transaction limit. 3. The recipient is blacklisted by the XVS token. 4. The minting cap on the destination bridge is exceeded. Specifically, if the source bridge is not configured as a trusted remote on the destination chain, the cross-chain messages from the source bridge will be blocked. A privileged role has to add or update the trusted remotes on the destination chain and call `retryPayload()` on LayerZero's Endpoint to retry the blocked message. Note that the `LzApp.forceResumeReceive()` function will drop the blocked message without retrying forwarding it to the destination bridge, which should only be used in extreme circumstances as it can potentially cause loss of funds.
Open the reportShow 10 more
LowPotential EVM Compatibility Issue in Solidity Verison >=0.8.20Venus Multichain Support · Quantstamp · 19 Dec 2023Fixed
According to the Hardhat config file, the `VTreasuryV8` contract is compiled in Solidity version 0.8.20 with the EVM version set as the default Shanghai version. As a result, the contract bytecode may include `PUSH0` opcodes, which are not supported on certain EVM chains, including BSC, [Arbitrum](https://docs.arbitrum.io/solidity-support), [Optimism](https://community.optimism.io/docs/developers/build/differences/), and opBNB. The deployment or execution of the contract may fail due to the use of an invalid opcode.
Open the reportLowPotential of Locked Funds in the XVS Bridge Admin ContractVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The `XVSBridgeAdmin` contract has a `payable` fallback function without a withdraw function. Also, there is no native token provided in the call to the XVS bridge contract. As a result, any native tokens sent in a fallback function call will be locked in the admin contract.
Open the reportLowUse of Solidity's `transfer()` FunctionVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
Solidity's `transfer()` function forwards a fixed amount of 2300 gas to the recipient when transferring native tokens to them, which causes several limitations if the recipient is a contract. First, the limited amount of gas prevents the contract from executing more complicated logic (e.g., external calls) in its `fallback()` or `receive()` function. Second, the gas costs of each opcode are subject to change, and therefore, it is not guaranteed that currently successful transfers will not fail in the future. The `VTreasury.withdrawTreasuryNative()` allows the owner to withdraw native tokens from the Venus Treasury, which uses Solidity's `transfer()` function to transfer native tokens to a designated address.
Open the reportLowXVS Bridges May Still Receive or Release Tokens When PausedVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The `owner` of the XVS bridge can pause or unpause the bridge. When the bridge is paused, function calls to `_debitFrom()` and `_creditTo()` will revert. The `_debitFrom()` function is called when a user initiates a transfer on the source chain, and the `_creditTo()` function is called when the user receives the transferred token on the destination chain. No user can initiate a transfer from the source bridge to others when a source bridge is paused. However, when a destination bridge is paused, the following scenarios are still possible: 1. The destination bridge can receive cross-chain messages, which attempt to mint or release tokens to the receiver. Although these messages will fail, they can be retried once the destination bridge is unpaused. 2. Messages of type `PT_SEND_AND_CALL` will trigger an `onOFTReceived()` function call to the recipient. If the call fails, the messages will be stored, but the XVS tokens will be credited to the bridge. These messages can be retried during the pause period since they do not execute the `_creditTo()` but the `_transferFrom()` function. As a result, the recipient will still be able to receive the tokens during the pausing period.
Open the reportLowXVS Token's Name and Symbol Are ReversedVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The `XVS` token inherits from the `ERC20` contract, initialized by `ERC20("XVS", "Venus XVS")`. The first parameter is the `name` of the ERC-20 token, while the second is the `symbol`. As a result, the `XVS` token will have the name `XVS` and the symbol of `Venus XVS`.
Open the reportLowXVS Transfer Amount from Whitelisted Users Is Considered in Daily LimitVenus Multichain Support · Quantstamp · 19 Dec 2023Fixed
The `_isEligibleToSend()` and `_isEligibleToReceive()` functions in the `BaseXVSProxyOFT` contract allow a whitelisted user to send or receive arbitrary token amount, which is not restricted by the single-transaction and daily limits. The bridges will still update the `chainIdToLast24HourTransferred` mapping to account for the transfer value in USD from whitelisted users. As a result, large transfers from whitelisted users may block regular users' transfers within the same daily window if the whitelisted user's transaction gets executed first. On the other hand, regular users will not be affected if their transactions are executed before the whitelisted users. The execution order of transactions leads to different outcomes. This issue is marked as Infomational as its validity and impact depend on the expected protocol behavior.
Open the reportLowInconsistent Convention for Checking Access AllowanceVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportLowInconsistent Zero-Address ChecksVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportLowLack of Storage GapVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportLowMissing Check of XVS Store AddressVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportShow 10 more
LowMissing DocstringsVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportLowMissing Event EmissionsVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportLowOwner Can Only Sweep Tracked Assets From Risk FundVenus Token Converter Audit · OpenZeppelin · 27 Oct 2023Fixed
The report gives no description here.
Open the reportLowInconsistency of Error Handling for vToken's Privileged FunctionsVenus Income Allocation · Quantstamp · 12 Oct 2023Fixed
In the `VToken.setAccessControlManager()` function of the core pool, the caller is checked against the `admin`. If the caller is not authorized, the function returns an error code without reverting the transaction. However, the function comments state: > @return uint 0=success, otherwise will revert This inconsistency between the function's behavior and comments can confuse developers. Relying solely on the function's comments may lead developers to assume that a transaction will revert upon failure, while it may silently fail if `setAccessControlManager()` is called by an unauthorized caller. Additionally, other privileged setters within the codebase also have inconsistent behavior. For example, `setProtocolShareReserve()` silently fails with an error code returned if the caller is unauthorized, while `setReduceReservesBlockDelta()` reverts the transaction during an unauthorized call.
Open the reportLowIt Is Not Possible to Remove Items From the `distributionTargets` ArrayVenus Income Allocation · Quantstamp · 12 Oct 2023Fixed
Currently, the distribution configs are updated via the `addOrUpdateDistributionConfigs()` function, and hence, the `distributionTargets` array is updated. It is currently possible to add an entry or update a percentage of existing entries but not remove entries from the array, hence leading to an issue where if an entry was added incorrectly, it could not be removed and only be set to 0 percentage, hence leading to be not accounted for, but would waste gas.
Open the reportLowSafe Math Function Not Used For Block Delta CalculationVenus Income Allocation · Quantstamp · 12 Oct 2023Fixed
The `accrueInterest()` function in the Core Pool `VToken` contract uses a library to perform arithmetic calculations due to the older Solidity version. The new implementation of this function, which adds the ability to call `_reduceReservesFresh()` if enough blocks have passed since it was last called, performs a calculation without using the library. While this should not be an issue, as the block number is steadily increasing, it is recommended to match previous standards.
Open the reportLowImplementation of EIP-2535 Does Not Fully Match the SpecificationVenus Protocol Diamond Comptroller Audit · OpenZeppelin · 28 Aug 2023Fixed
The report gives no description here.
Open the reportLowIncorrect Function Signature in _setActionsPausedVenus Protocol Diamond Comptroller Audit · OpenZeppelin · 28 Aug 2023Fixed
The report gives no description here.
Open the reportLowMissing DocstringsVenus Protocol Diamond Comptroller Audit · OpenZeppelin · 28 Aug 2023Fixed
The report gives no description here.
Open the reportLowUnnecessary Access Allowance to the Comptroller ImplementationVenus Protocol Diamond Comptroller Audit · OpenZeppelin · 28 Aug 2023Fixed
The report gives no description here.
Open the reportShow 10 more
LowOther Protocols May Assume There Is A Single `RewVenus - Isolated Pools Allow Multiple Rewards Dist · CertiK · 24 Aug 2023Fixed
There may be protocols that interact with Venus Isolated-Pools that based their design on the assumption that there is a single `RewardsDistributor` per reward token. For example, they may use a mapping from the reward token to the `RewardsDistributor`, which will not allow them to handle scenarios where there are multiple rewards distributors.
Open the reportLowPossible To Have Multiple Rewards Distributors IsVenus - Isolated Pools Allow Multiple Rewards Dist · CertiK · 24 Aug 2023Fixed
Currently it is possible to add two distinct rewards distributors that will have the same reward token and have an overlap in the blocks in which the rewards will be distributed. As the reward token speeds and last reward block can be adjusted, a single rewards distributor can be used in the case that there is an overlap in rewards.
Open the reportLowOwnership Can Be RenouncedVenus Protocol V2 · Quantstamp · 27 Jul 2023Fixed
If the owner renounces his ownership, the contract will be left without an owner. Consequently, it will no longer be possible to execute functions guarded by the `onlyOwner` modifier.
Open the reportLowMissing Input ValidationVenus Liquidator · Quantstamp · 26 Jul 2023Fixed
It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. The function `setPendingRedeemChunkLength()` should check that `newLength_` is not 0.
Open the reportLowOwnership Can Be RenouncedVenus Liquidator · Quantstamp · 26 Jul 2023Fixed
The Liquidator contract inherits `Ownable2StepUpgradeable`, which allows having an owner of the contract. It also features a function allowing to renounce owner role via `renounceOwnership()`. If the owner renounces their ownership, any function guarded by the `onlyOwner` modifier will no longer be able to be executed.
Open the reportLow`markets()` Does Not Use All Return ValuesFixed
The function `markets()` in the comptroller returns a triple `(bool, uint256, bool)`, however the interface only has a `bool` return value. Currently this will function as intended as it will reference the correct memory to return the first `bool` in the triple which corresponds to if the market is listed. However, it is possible that in future iterations of the virtual machine that this behavior will not remain the same.
Low`Natspec` Comments Missing ParametersFixed
The `natspec` comments of many functions cited above are missing the parameter `deadline`. The `natspec` comment for the function `sweepToken()`, is missing the parameter `to`.
Low`ReentrancyGuardUpgradeable` Is Not InitializedFixed
Contract `PegStability` extends ReentrancyGuardUpgradeable, while `__ReentrancyGuard_init()` is not called in the initialize function. Generally, the initializer function of an upgradeable contract should always call all the initializer functions of the contracts that it extends.
Low`require` statement can be substituted with a `moFixed
The `require` statements on the aforementioned lines can be substituted with a `modifier` to increase the legibility of the codebase and to avoid code duplciation.
Low`require` statement can be substituted with a `moFixed
The `require` statements on the aforementioned lines can be substituted with a `modifier` to increase the legibility of the codebase and to avoid code duplication.
Show 10 more
Low`require` statement can be substituted with a `moFixed
The `requrie` statement on the aforementioned lines can be substituted with a `modifier` to increase the legibility of the codebase and also to avoid code duplication.
Low`VRTVault` May Be Operated and Used Past the Last Accruing BlockFixed
The `VRTVault` has a built-in deprecation through variable `lastAccruingBlock`, which dictates after which block number no more rewards will be accrued. There are three scenarios when and how it can be changed: 1. When calling it the first time it can be set arbitrarily in the future or the past. 1. Once initialized but not yet reached, it can be set to an arbitrary block number in the future, or a value between the current block number and the current value of `lastAccruingBlock`. 1. **Once reached/passed it, it can be changed to be at an arbitrary number in the past or the future and thereby re-enable `deposit()`.**
LowAccommodation of Non-ERC20-Compliant TokensFixed
The report gives no description here.
LowAssumption On Same DecimalsFixed
Note that on BSC all tokens USDT, USDC, and VAI have 18 decimals so there is no issue with the logic. However, if stable tokens with other decimals are used, then the logic must be adjusted to account for the difference in decimals.
LowBoolean EqualityFixed
The report gives no description here.
LowBoolean EqualityFixed
The report gives no description here.
LowCan Use `safeTransfer()`Fixed
When transferring `WBNB` the call is checked to be successful via an assert statement. However, the `TransferHelper` library's functions can be used in place of this.
LowCan Use Single Address InputFixed
The functions `_checkForAmountOut()` and `_getSwapAmount()` have an input array of addresses `path`, however, only the last element is used. A single address can instead be taken as input to reduce gas costs. In addition, the last element of the path `path[path.length - 1]` is used multiple times in functions, storing this in a temporary variable and referencing the temporary variable can save gas.
LowCode DuplicationFixed
The report gives no description here.
LowComments For Functions Supporting FeeFixed
The comments above some of the functions that support fee on transfer tokens in the contract `SwapRouter` do not mention how it is intended to be used for fee on transfer tokens.
Show 10 more
LowComparison to Boolean ConstantFixed
Boolean constants can be used directly and do not need to be compared to true or false. ```solidity=85 require(isPaused == false, "Contract is paused."); ``` --- ```solidity=247 require(isPaused == false, "PSM is already paused."); ``` --- ```solidity=259 require(isPaused == true, "PSM is not paused."); ```
LowComparison with a literal `false`Fixed
The aforementioned lines perform comparison with a literal false which is unnecessary and can be replaced with the negation of expression f.e `!expression` .
LowComparison with a literal `true`Fixed
The aforementioned lines perform comparison with a literal `true` which is unnecessary and can be replaced with the expression itself.
LowContract Should Be a LibraryFixed
The report gives no description here.
LowContradictionFixed
The report gives no description here.
LowCritical Role Transfer Not Following Two-Step PatternFixed
The `admin` role can be transferred to another address simply by calling the `VAIVault.setNewAdmin()` function. This function immediately transfers a high-level privilege to new addresses in a single transaction, which can be risky from a security perspective, as providing a faulty address may lock out that role from future calls. A more secure pattern for such privilege transfers is to require the new pending addresses to issue an `acceptAdmin()` function call before finalizing the transfer. Note that this pattern is common even with the use of timelocks, such as the [Compound Timelock](https://github.com/compound-finance/compound-protocol/blob/master/contracts/Timelock.sol) contract.
LowCustom Errors Can Be UsedFixed
Custom Errors are used throughout the codebase, however, string errors are still used in the codebase.
LowDocumentation discrepancyFixed
The comment on the aforementioned line refers the variable following it as `borrow index` whereas it is infact a `supply index` .
LowDuplication of Well-Known ContractsFixed
The report gives no description here.
LowEnvironmental ConsistencyFixed
The report gives no description here.
Show 10 more
LowFee Amount Not Checked In InitializerFixed
The input `feeIn_` and `feeOut_` of `initialize()` function are not checked to be less than `1000`.
LowFloating PragmaFixed
The report gives no description here.
LowFloating PragmaFixed
The report gives no description here.
LowFloating PragmaFixed
The report gives no description here.
LowFunction Visibility OptimizationFixed
The linked function is declared as public , contains array function arguments and is not invoked in any of the contract's contained within the project's scope.
LowFunctions that Can Be Declared ExternalFixed
The report gives no description here.
LowInaccurate Calculation of Pending Rewards of XVS Vault PoolsFixed
According to the design of the XVS vault, pending withdrawals after the upgrade are not eligible for accruing rewards. However, the `pendingReward()` function includes every pending withdrawal when calculating the pending rewards. This issue would affect the front end and any third-party protocols integrating with the XVS vault using this function.
LowIncomplete CheckFixed
The following check is performed in the function `getAmountIn()` and `getAmountOut()`: ```solidity if (reserveIn == 0 && reserveOut == 0) { revert InsufficientLiquidity(); } ``` However, this check does not cover the case when either `reserveIn` or `reserveOut` is 0, while the other is nonzero.
LowIncorrect CommentFixed
The comment above `sweepToken` states that tokens are sent to admin (timelock). However, the tokens are sent to the input `to` address, which may not be the admin.
LowIncorrect grammarFixed
The comment on the aforementioned line has incorrect grammar for the word borrow .
Show 10 more
LowIncorrect Old Price OracleFixed
In the function `setPriceOracle()`, the `oldPriceOracleAddress` is set to be the input `priceOracle_`. However, it should be the current `priceOracle`.
LowIncorrect order of functionsFixed
The structure of the codebase does not conform to the official Solidity style guide of `v0.5.16` . The style guide can be found here https://docs.soliditylang.org/en/v0.5.16/style-guide.html#or der-of-functions.
LowIneffectual comparison with `address(0)`Fixed
The comparison `msg.sender == address(0)` is ineffectual as it will never evaluate to `true` because `msg.sender` can never have a value of `address(0)` .
LowIneffectual comparison with `address(0)`Fixed
The comparison `pendingVAIControllerImplementation == address(0)` on the aforementioned line is ineffectual as it will never evaluate to `true` . When the first comparison evaluates to `false` in the event of `pendingVAIControllerImplementation` being equal to `msg.sender` then the aforementioned comparison will always evaluate to `false` as `msg.sender` can never can be equal to `address(0)` .
LowInefficient `storage` readFixed
The aforementioned line perform unnecessary storage read as the same value can be read from a local variable of `newCloseFactorMantissa`.
LowInefficient `storage` readFixed
The storage read on the aforementioned line is unnecessary as the same value is available through local variable `newPauseGuardian`.
LowInefficient ChecksFixed
The following two checks are performed: ```solidity require(reserveA > 0 && reserveB > 0, "PancakeLibrary: INSUFFICIENT_LIQUIDITY"); ``` which will revert if `reserveA` or `reserveB` is zero. ```solidity if (reserveA == 0 && reserveB == 0) { revert InsufficientLiquidity(); } ``` which will revert if `reserveA` and `reserveB` are zero. Thus this check is not needed as the require statement covers it.
LowInefficient codeFixed
The type of function parameter `vToken` on `L1021` can be changed to VToken to avoid unnecessary casting to address type and save gas cost associated with it.
LowInefficient Event ParameterFixed
The event `VaiMintCapChanged` uses the storage variable `vaiMintCap`. However, gas can be saved by using `vaiMintCap_`.
LowInefficient For LoopFixed
The function `_reduceReservesInternal()` has a for loop that starts with the `index` at `range - 1` and decreases by 1 as long as the `index` is greater than or equal to 0. Thus at the final step the `index = 0` is decremented so that it must be a signed integer. However, in the logic of the for loop, the index is only used when it is an unsigned integer and the loop can be refactored to only use unsigned integers.
Show 10 more
LowInefficient function declarationsFixed
The functions on the aforementioned lines perform same functionality with only difference being one of them provides a public interface while the other one is an internal implementation.
LowInefficient Greater-Than Comparison w/ ZeroFixed
The linked greater-than comparisons with zero compare variables that are restrained to the non-negative integer range, meaning that the comparator can be changed to an inequality one which is more gas efficient.
LowInefficient local variableFixed
The local variable on the aforementioned line is only used once in the code and its usage can be replaced with initialization part of the statement on `L135` to save gas cost associated with local variable declaration.
LowInterface Marked As An Abstract ContractFixed
The report gives no description here.
LowLack Of Input ValidationFixed
The function `liquidateBorrow()` takes an input `vToken` and `vTokenCollateral` which are never checked to be supported `vToken`. Thus a malicious user can input a malicious contracts address for them so that it will fail on redemption and `vTokenCollateral` will be added to the `pendingRedeem` array. This leads to a potential denial of service as enough `vToken` can be added to the array so that other pending redemptions will not be executed. In addition, this allows an entry point for a hacker as whenever `liquidateBorrow()` is called, it will call `_reduceReservesInternal()` which will make an external call to a potentially malicious address.
LowLocked BNB in ContractFixed
The report gives no description here.
LowLogic Issue of Comptroller::_setContributorVenusSpeed()Fixed
The report gives no description here.
LowMisleading DocumentationFixed
The report gives no description here.
LowMissing ChecksFixed
It is checked that the input `vTokenAddress` is listed, however, there is no check that the final element of the input `path` is the underlying of the `vToken`. As approval is given for the last element of the input `path` to either repay or supply, it should be checked that it corresponds to the underlying of the `vToken`. It is not checked that the input `vBNBAddress` is listed. It should be checked that this is a listed market, or alternatively, the input can be removed and the address stored by the contract itself.
LowMissing DocstringsFixed
The report gives no description here.
Show 10 more
LowMissing License IdentifierFixed
The report gives no description here.
LowMissing Zero Address ValidationFixed
The report gives no description here.
LowMissing Zero Address ValidationFixed
In the `constructor()`, the input `_comptrollerAddress` is not checked if it is the zero address.
LowMissing Zero Address ValidationFixed
The report gives no description here.
LowMultiple Functions To Set Access Control ManagerFixed
The function `setAccessControl()` and the inherited function `setAccessControlManager()` for `AccessControlledV8` both call the inherited internal function `_setAccessControlManager` to set the `_accessControlManager`.
LowOutdated Chainlink InterfaceFixed
The report gives no description here.
LowOutdated Solidity VersionFixed
The report gives no description here.
LowPool May Be Permanently MisconfiguredFixed
The function `_updatePool()` divides by `totalAllocPoints[_rewardToken]` when calculating the reward. This variable is updated in the `add()` function: ``` totalAllocPoints[_rewardToken] = totalAllocPoints[_rewardToken].add(_allocPoint); ``` `_allocPoint` must be greater than zero the first time a pool is added. Otherwise, `_updatePool()` would revert due to division by zero, which would block users from calling `add()`, `deposit()`, `claim()`, and `requestWithdrawal()` for the corresponding `_rewardToken`. A blocked call to `massUpdatePools()` will prevent fixing the configuration by calling `set()`.
LowPossible ReentrancyFixed
The functions `liquidateBorrow()` and `reduceReserves()` are external and can be called by anyone. While `liquidateBorrow()` has the `nonReentrant` modifier, `reduceReserves()` does not and may allow an attacker to re-enter the contract. In particular, the external calls made to the inputs `vToken` and `vTokenCollateral` of `liquidateBorrow()` may be used to call `reduceReserves()` and re-enter the contract. *This finding is considered minor because the reentrancy only causes out-of-order events.*
LowPossible Underflow RevertFixed
As VAI can also be minted by borrowing against assets, there is more VAI in circulation than the value stored in `vaiMinted`. If a user attempts to call `swapVAIForStable()` so that the `stableTknAmountUSD` is greater than `vaiMinted` and `vaiMinted` is nonzero, then the call will revert with a non-descriptive error.
Show 10 more
LowPotential Reentrancy (Out-of-Order Events)Fixed
A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. * Reentrancy can occur during the swaps as they will make external calls to the receiver of the tokens, this will cause the corresponding swap events to be emitted out of order. * If a token implements hooks that make external calls, then reentrancy can also occur during token transfers, which may cause events to be emitted out of order. * Token addresses are also provided as inputs, so that it is possible that a token is provided that can cause reentrancy with any external call made to it (for example if `balanceOf`), which may cause events to be emitted out of order. *This finding is considered minor because the reentrancy only causes out-of-order events.*
LowProper dsrPerBlock() CalculationFixed
The report gives no description here.
LowPublic Functions That Should Be ExternalFixed
The report gives no description here.
LowRedundant CheckFixed
The report gives no description here.
LowRedundant ContractFixed
The report gives no description here.
LowRedundant FunctionFixed
The report gives no description here.
LowRedundant ImportFixed
The report gives no description here.
LowRedundant State/Code RemovalFixed
The report gives no description here.
LowRedundant StatementsFixed
The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.
LowRedundant UseFixed
The report gives no description here.
Show 10 more
LowRedundant Variable InitializationFixed
All variable types within Solidity are initialized to their default "empty" value, which is usually their zeroed out representation. Particularly: - uint / int : All uint and int variable types are initialized at 0 - address : All address types are initialized to address(0) - byte : All byte types are initialized to their byte(0) representation - bool : All bool types are initialized to false - ContractType : All contract types (i.e. for a given contract ERC20{} its contract type is ERC20) are initialized to their zeroed out address (i.e. for a given contract ERC20{} its default value is ERC20(address(0))) - struct : All struct types are initialized with all their members zeroed out according to this table
LowReturned error is not checkedFixed
The function call on the aforementioned line returns an error code which is not checked. The problem will arise if the `vaiController` is not set in the `comptroller` contract then the function call will still be treated as successful but the mint amount would not have been set.
LowReturned error is not checkedFixed
The function call on the aforementioned line returns an error code which is not checked. The problem will arise if the `vaiController` is not set in the `comptroller` contract then the function call will still be treated as successful but the mint amount would not have been set.
LowSPDX License Identifier Not ProvidedFixed
The report gives no description here.
LowStyle Guide ViolationFixed
The report gives no description here.
LowStyle Guide Violation - Order of LayoutFixed
The report gives no description here.
LowSuggested Address Validity CheckFixed
The report gives no description here.
LowTyposFixed
In the function `_distributeLiquidationIncentive()`, `siezedAmount` should be spelled as `seizedAmount`.
LowTypos And InconsistenciesFixed
In the function `swapVAIForStable`, "enough" is misspelled as "enought". The error message "Amount must be greater than zero" does not use a period as all other error messages do.
LowUnchecked Blocks Can Optimize ContractFixed
In the function `reduceReservesInternal()`, the for loop `index` can be decremented inside an unchecked block to save gas as `index >= 0` ensures it will not underflow.
Show 10 more
LowUnchecked Blocks Can Optimize ContractFixed
In the function `swapStableForVAI`, the check ```solidity require(vaiMinted + actualTransferAmtInUSD <= vaiMintCap, "VAI mint cap reached."); ```` is preformed before incrementing `vaiMinted` by the `actualTransferAmtInUSD`. As the check adds `vaiMinted` and `actualTransferAmtInUSD` it already checks for overflow. Thus it does not need to be checked when incrementing.
LowUnchecked Blocks Can Optimize ContractFixed
In general, the counter in a for loop can be incremented or decremented in an unchecked block.
LowUndocumented LogicFixed
The report gives no description here.
LowUnindexed EventsFixed
The report gives no description here.
LowUnused Custom ErrorFixed
The custom error `UnderlyingTransferFailed` is declared but never used.
LowUnused Storage VariableFixed
The report gives no description here.
LowUnused Try/Catch ParameterFixed
The report gives no description here.
LowUse of Hard-Coded ValuesFixed
The report gives no description here.
LowUsing AbiCoderV2 in Solidity ^0.8.0Fixed
The report gives no description here.
LowVariable ShadowingFixed
The report gives no description here.
Incidents2
Attached through a shared contract address. Our incident data ends in Oct 2026.
Incident
Incident,
Source: De.Fi incident database.
- Audits before it
- 1.
- Audits since
- 66, the latest dated 13 Sep 2025.
Contracts2
| Address | Chain | Source code |
|---|---|---|
| 0xd3cc9d8f3689b83c91b7b59cab4946b063eb894a | Chain: Ethereum | Source code: Verified source |
| 0x67c549a18abfad127b13f8d56738f43a21bb62a7 | Chain: BNB Chain | Source code: Verified source |
Show this on your site
Is this your project? Show this record on your own website. The label states what the record holds, links to this page and updates by itself. It is free and needs no sign-in.
Content
Automatic: the three strongest facts of this project’s own record (its last audit and two more), highlighted. They update with the record. Picking a fact turns Automatic off.
Style
Format
<head><script src="https://widget.trustblock.run/project-widget.min.js" defer></script></head><body><divdata-tb-project-widgetdata-tb-project-slug="venus-protocol"></div></body>
Paste the code where the label should appear. It uses no API credit and updates by itself.
Something wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us