Velvet Capital

The Rebalancing contract allows the Asset Manager to influence the Portfolio's configuration, namely the distribution of tokens inside of the portfolio. Additionally, it is also possible to use the updateTokens function to update the tokens themselves that a given Portfolio consists of. It was observed that the updateTokens function is publicly callable by anyone, not just by the Asset Manager. This fact, along with other minor issues, leads to draining the Portfolio contract from its assets.The whole updateTokens flow consists of:Checking if supposed new tokens are whitelisted (if whitelisting is enabled).Checking if the number of new tokens exceeds the asset number limit.Updating the token weights, by pulling previous tokens from Portfolio's vault and selling them using the swap target (via Enso protocol).Ensuring that the tokens were swapped for the expected price (done in the EnsoHandler contract).Ensuring that all of the tokens pulled from Portfolio's vault were sold, i.e. the EnsoHandler contract has a balance of 0 for each token present previously in the Portfolio's vault.Ensuring that the Portfolio's vault has a positive balance of each new token.Ensuring that the Portfolios vault has a zero balance for each token that was removed from the Portfolio.The flow also requires that a legitimate, Velvet-deployed handler contract is used in the process, but the scenario described below uses the legitimate handler so this requirement was omitted.The input parameters for the updateTokens function are not checked if they are internally consistent. Specifically, the tokens used by EnsoHandler contract to ensure successful swap can be different than the actual tokens received from the swap operation.If token whitelisting is disabled, then the malicious user might create own token contracts (or any contract that will implement balanceOf and transfer function with accordance to ERC20 standard). Providing those contracts' addresses as _newTokens and as tokens inside of the _callData in the updateTokens function will allow the malicious user to control the checks from points 4 and 6 of the flow described above.Furthermore, the updateTokens caller also controls the callDataEnso argument, which is essentially an encoded call to the Enso protocol that dictates the swapping. Hence, the attacker can use it to instruct the Enso protocol to sell all of the tokens previously held in the Portfolio for any legitimate other token supported by the swap. This will assure that checks from points 5 and 7 are fulfilled. If token whitelisting is enabled, then a malicious user needs to use the legitimate token that is whitelisted. Before calling the updateTokens malicious user would need to transfer some tokens (even 1) to make the Portfolio's vault balance non-zero for those whitelisted tokens. That way, the check from point 6 is fulfilled. Those legitimate tokens' addresses would need to be provided as _newTokens, and custom malicious contract would be provided as tokens embedded in the _callData variable. As a consequence, a malicious user can force the protocol to sell all of the legitimate tokens and essentially lock them in the handler contract causing severe capital loss.Furthermore, the Enso protocol's contract used by the Velvet's swap handler was investigated. It appears that currently it does not support swapping via delegatecall. However, should this feature be enabled, then this finding would result in a token theft, as the attacker could use the callDataEnso to initiate a delegatecall to the contract that he controls. As the delegatecall mechanism preserves the values for msg.sender then the token transfer could be initiated to steal the tokens instead of swapping them and locking the swapped for tokens inside of the handler contract.

The function _authorizeUpgrade() is used in OpenZeppelin's Upgradeability design patterns to provide developers a possibility of enforcing access control during upgrades of the smart contracts. By default, smart contracts that inherit this upgradeability design can be upgraded by any user without any access control implemented.The usage of the function _authorizeUpgrade() on the protocol does not enforce proper access control on which specific users may upgrade the smart contract. As such, any malicious user may use this opportunity to upgrade the smart contract with malicious code and ultimately be able to compromise and takeover the entire protocol.

The Portfolio contract serves as the primary entry-point for users to interact with the protocol. Among other functionalities, it defines a receive endpoint that is executed whenever a call with empty calldata is received. The Portfolio contract will accept any form of regular Ether transfers. However, it was observed that it does not implement any way for user to receive Ether from the contract. As a consequence, any Ether transfer to the Portfolio contract will result in a permanent asset lost for the user. Similarly, the same receive function is implemented by the EnsoHandler and EnsoHandlerBundled contracts, which are used as a intermediary between the Velvet protocol and Enso protocol, by the Rebalancing contract and by the TokenExclusionManager.

An implementation contract should not be left uninitialized. An uninitialized implementation contract can be taken over by an attacker, which may impact the proxy.Several implementation instances across the protocol do not disable contract initialization with the usage of the function _disableInitializaers().

The function setProtocolPause() does not validate if the protocol is in emergency pause before unpausing. This means that it is possible for the protocol to be incorrectly unpaused at will, which will trigger many instances of undefined behaviour across the protocol where certain functionalities only check for either isProtocolPaused or isProtocolEmergencyPaused and not both.

This vulnerability arises in the functions updateProtocolFee and updateProtocolStreamingFee of the smart contract contracts/config/protocol/ProtocolFeeManagement.sol:In current actual shape, the function permits to update protocol fees to 100% by the protocol Owner, allowing potentially malicious contract owners or authorized users to configure fees that could seize the entirety of a transaction's value. This can lead to fraudulent revenue generation, severely impacting user trust and the economic stability of the platform.If an attacker gains control over the contract owner's account (through phishing, private key compromise, etc.) and sets the transaction and protocol fees to 100%. Consequently, every transaction performed on the platform results in users losing all their transferred funds to fees, effectively rendering the platform unusable and draining user assets. This not only disrupts service but also destroys user confidence and could potentially lead to significant financial losses for all stakeholders involved.

The Velvet Capital contracts are often using arrays as parameters for their functions. Those arrays are usually looped over to execute certain actions. However, it was observed that in some cases, there checks to make sure that the arrays necessary for a given execution flow have matching lengths. As a result, it is possible to provide arrays with different lengths that will make the execution fail when EVM will try to access an element that does not exist in an array.The missing checks were identified for contracts:EnsoHandlerBundled - missing check on tokensLength and callDataEnso.length in multiTokenSwapAndTransfer function.Rebalancing - missing check on sellAmounts.length and sellTokens.length in _updateWeights function.

The variables transferable and transferableToPublic are not validated during the contract deployment and may lead to bricking the smart contract with incorrect initialization values, requiring it to be deployed again in order to be fixed. This is due to the validations being made on the function updateTransferability().

The FeeManagement contract is responsible for holding values for various fees associated with using the protocol. It also implements a functionality to change values of those fees. The update mechanism is a 2-step process that uses a time period until the second step, actual fee change, can be executed. Using such a time period is considered a good practice. However, it was observed that after one successful update, the same update function can be called indefinitely, until new fee proposal is created. Such subsequent update calls wouldn't change the fee, however they would still emit an event which might disrupt operation of any entity that uses this data.This finding is related to the updateManagementFee, updatePerformanceFee and updateEntryAndExitFee functions.

The PriceOracle contract responsible for fetching prices from Chainlink's oracles implements some checks to make sure that the price returned by the Chainlink oracle is valid. One of those checks is to ensure that it is not equal to 0. However, Chainlink oracles contain variety of information, not necessarily the price of an asset with regards to another asset. This is why, the answer returned by Chainlink's oracle is of type int256. Although it is very unlikely for an asset price oracle to return a negative price, the check if it is equal to 0 would not catch this case.