hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

Expensive Operation on `updateTokens` #113

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x227834fdffe74832b3d90051b5e9b71f198a4e5f21b40e4e99355a3304f53f49 Severity: medium

Description: Description\ DOS can be caused on updating tokens and weights as the memory complexity of update WEIGHT is 2N + 3M, where N is sellTokens array length and M is _newTokens length.

On update

Attack Scenario\ DOS can be caused on updating tokens and weights as the memory complexity of update WEIGHT is 2N + 3M, where N is sellTokens array length and M is _newTokens length.

On updating tokens, extra loops is introduced to are a round trip from updateWeights, introducing extra 3 loops, assuming sellTokens and newTokens length are same length of N, complexity is now 8N.

This can introduce expensive and hard to run transactions

Attachments

  1. Proof of Concept (PoC) File A spread out updateTokens function gives

    
    unction updateTokens(
    FunctionParameters.RebalanceIntent calldata rebalanceData
    ) external virtual nonReentrant onlyAssetManager {
    address[] calldata _sellTokens = rebalanceData._sellTokens;
    address[] calldata _newTokens = rebalanceData._newTokens;
    address[] memory _tokens = _getCurrentTokens();
    
    //Need a check here to confirm _newTokens has buyTokens in it
    portfolio.updateTokenList(_newTokens);
    
    // Perform token update and weights adjustment based on provided rebalance data.
    if (protocolConfig.isProtocolPaused())
      revert ErrorLibrary.ProtocolIsPaused();
    
    if (!protocolConfig.isSolver(_handler)) revert ErrorLibrary.InvalidSolver();
    
    // Pull tokens to be completely removed from portfolio to handler for swapping.
    uint256 sellTokenLength = _sellTokens.length;
    uint256 sellAmountLength = _sellAmounts.length;
    
    if (sellAmountLength != sellTokenLength) {
      revert ErrorLibrary.InvalidLength();
    }
    
    for (uint256 i; i < sellTokenLength; i++) {
      address sellToken = _sellTokens[i];
      if (sellToken == address(0)) revert ErrorLibrary.InvalidAddress();
      portfolio.pullFromVault(sellToken, _sellAmounts[i], _handler); // @audit sellamount must be equal to balance of handler
    }
    
    // Execute the swap using the handler.
    address[] memory ensoBuyTokens = IIntentHandler(_handler)
      .multiTokenSwapAndTransfer(_vault, _callData);
    
    // Verify that all specified sell tokens have been completely sold by the handler.
    for (uint256 i; i < sellTokenLength; i++) {
      if (_getTokenBalanceOf(_sellTokens[i], _handler) != 0)
        revert ErrorLibrary.BalanceOfHandlerShouldBeZero();
    }
    
    // Ensure that each token bought by the solver is in the portfolio list.
    uint256 tokenLength = _newTokens.length;
    for (uint256 i; i < tokenLength; i++) {
      address token = _newTokens[i];
      if (_getTokenBalanceOf(token, _vault) == 0)
        revert ErrorLibrary.BalanceOfVaultCannotNotBeZero(token);
      tokensMapping[token] = true;
    }
    
    // Validate that all tokens bought by the solver are in the list of new tokens.
    uint256 ensoBuyTokensLength = ensoBuyTokens.length;
    for (uint256 i; i < ensoBuyTokensLength; i++) {
      if (!tokensMapping[ensoBuyTokens[i]]) {
        revert ErrorLibrary.InvalidBuyTokenList();
      }
    }
    
    // Reset the tokens mapping for the new tokens.
    for (uint256 i; i < tokenLength; i++) {
      delete tokensMapping[_newTokens[i]];
    }
    
    // Update the internal mapping to reflect changes in the token list post-rebalance.
    uint256 tokenLength = _tokens.length;
    for (uint256 i; i < tokenLength; i++) {
      tokensMapping[_tokens[i]] = true; //@audit repeated update of tokenMapping L-1
    }
    
    uint256 newTokensLength = _newTokens.length;
    for (uint256 i; i < newTokensLength; i++) {
      tokensMapping[_newTokens[i]] = false;
    }
    
    for (uint256 i; i < tokenLength; i++) {
      address _portfolioToken = _tokens[i];
      if (tokensMapping[_portfolioToken]) {
        if (_getTokenBalanceOf(_portfolioToken, _vault) != 0)
          revert ErrorLibrary.NonPortfolioTokenBalanceIsNotZero();
      }
      delete tokensMapping[_portfolioToken];
    }
    
    emit UpdatedTokens(_newTokens);
    }


2. **Revised Code File (Optional)**
<!-- If possible, please provide a second file containing the revised code that offers a potential fix for the vulnerability. This file should include the following information:
- Comment with a clear explanation of the proposed fix.
- The revised code with your suggested changes.
- Any additional comments or explanations that clarify how the fix addresses the vulnerability. -->
langnavina97 commented 1 week ago

The complexity of the updateTokens function is indeed high, but it is necessary to ensure that all tokens are correctly sold, bought, and validated during the rebalancing process. These checks are essential for maintaining the integrity of the rebalancing process and ensuring that the portfolio tokens are correctly managed.