Venombridge security record

```solidity=768 /** @notice Change the quantity of assets `strategy` may manage. This may be called by `governance` or `management`. @param strategyId The Strategy to update. @param _debtRatio The quantity of assets `strategy` may now manage. */ function updateStrategyDebtRatio( address strategyId, uint256 _debtRatio ) external override onlyGovernanceOrManagement strategyExists(strategyId) { StrategyParams memory strategy = _strategy(strategyId); _debtRatioReduce(strategy.debtRatio); _strategyDebtRatioUpdate(strategyId, _debtRatio); _debtRatioIncrease(debtRatio); require(debtRatio <= MAX_BPS); emit StrategyUpdateDebtRatio(strategyId, _debtRatio); } ``` Function `updateStrategyDebtRatio()` is supposed to updates `debtRatio` (the total quantity of assets that all strategies can manage) by reducing `strategy.debtRatio` and increasing `_debtRatio`. However, the function call of `_debtRatioIncrease()` at line 787 mistakenly uses `debtRatio` as the parameter rather than `_debtRatio`, which would cause the value of `debtRatio` to be higher than the actual ratio that all strategies can manage. Since `debtRatio` cannot exceed 100% and the total ratio that all strategies can manage is smaller than `debtRatio`, the strategies cannot take full advantage of the assets in the vault contract.

Upgradeable contract must extend upgradeable contracts and call initializer functions of parent-contracts in its own initializer function. Otherwise, it may malfunction due to improper initialization or improper upgrading.

```solidity=830 function updateStrategyPerformanceFee( address strategyId, uint256 _performanceFee ) external override onlyGovernance strategyExists(strategyId) { require(_performanceFee <= MAX_BPS / 2); performanceFee = _performanceFee; emit StrategyUpdatePerformanceFee(strategyId, _performanceFee); } ``` Function `updateStrategyPerformanceFee()` updates the global variable `performanceFee` rather than `strategies_[strategyId].performanceFee`.

When developers deploy the implementation contract and call the function `initialize()` from the Proxy contract, the initialization will be executed in the context of the Proxy contract. Hence the state variable `_initialized` and `_initializing` will be 0 and false, respectively. However, the "initialize" function in the implementation contract is not initialized, and attackers can directly call the function `initialize(),` with malicious inputs to take over the implementation contract. Reference: https://medium.com/immunefi/wormhole-uninitialized-proxy-bugfix-review-90250c41a43a https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#initializing_the_implementation_contract

Based on code L72 in the `MultiVaultEverscaleEventAlien.sol` contract, `IProxyMultiVaultAlien_V3` should be used instead of `IProxyMultiVaultAlien_V1` at L140.

For upgradeable contracts, there must be a storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments". Otherwise, it may be very difficult to write new implementation code. Without a storage gap, the variable in the child contract might be overwritten by the upgraded base contract if new variables are introduced to the base contract. Refer to https://docs.openzeppelin.com/contracts/3.x/upgradeable#storage_gaps

Function `Vault.deposit()` at line 374 misses modifiers `onlyEmergencyDisabled` and `respectDepositLimit`, function `Vault.saveWithdraw()` at line 489 misses modifier `nonReentrant`, and function `MultiVault.deposit()` at line 623 misses modifier `onlyEmergencyDisabled`,

```solidity=1169 /** @notice Force user's pending withdraw. Works only if Vault has enough tokens on its balance. This may only be called by wrapper. @param pendingWithdrawalId Pending withdrawal ID */ function forceWithdraw( PendingWithdrawalId memory pendingWithdrawalId ) public override onlyEmergencyDisabled pendingWithdrawalOpened(pendingWithdrawalId) pendingWithdrawalApproved(pendingWithdrawalId) { PendingWithdrawalParams memory pendingWithdrawal = _pendingWithdrawal(pendingWithdrawalId); IERC20(token).safeTransfer(pendingWithdrawalId.recipient, pendingWithdrawal.amount); _pendingWithdrawalAmountReduce(pendingWithdrawalId, pendingWithdrawal.amount); emit PendingWithdrawalForce(pendingWithdrawalId.recipient, pendingWithdrawalId.id); } ``` `Vault.forceWithdraw()`'s comment says that the function may only be called by "wrapper", however, the role does not exist and anyone can call the function to withdraw a pending withdrawal to its recipient. `MultiVault.forceWithdraw()` shares the same issue, and it also misses modifiers like `nonReentrant` and `onlyEmergencyDisabled`.

`mergePoolVersion` is not included in upgrade data. Its value will be lost after the upgrade. Reference: https://docs.everscale.network/develop/smart-contract/learn/how-to-upgrade-contracts https://github.com/tonlabs/samples/blob/master/solidity/12_BadContract.sol and https://github.com/tonlabs/samples/blob/master/solidity/12_NewVersion.sol

```solidity modifier withdrawalNotSeenBefore(bytes memory payload) { bytes32 withdrawalId = keccak256(payload); require(!withdrawalIds[withdrawalId], "Vault: withdraw payload already seen"); _; withdrawalIds[withdrawalId] = true; } ``` The value of `withdrawalIds[withdrawalId]` is updated after `_;` runs, which violates the check-effect-interaction pattern.