hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

Vaultconfig's `_currentSnapshotId()` is neither initialized nor updated across the protocol #89

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: @burhankhaja Twitter username: imaybeghost Submission hash (on-chain): 0xa368a2695345cddf028cc558ffd6ecaa8b47ffc01f0730da1db9025449108b83 Severity: low

Description: Description\ Velvet protocol uses two types of _currentSnapshotId(), both with different purposes:

Note that the _currentSnapshotId of TokenExclusiveManager is properly updated and is used to track the snapshot id.

But on the otherhand, vaultConfig's _currentSnapshotId is intended to be tracking the No of token updates by the Rebalancing contract.

However, it is neither initialized nor is it updated anywhere across the whole protocol.

Recommendation\ increment _currentSnapshotId in updateTokenList(address[]) and initToken(address[])

  function updateTokenList(
    address[] calldata _tokens
  ) external onlyRebalancerContract {
    uint256 _assetLimit = protocolConfig().assetLimit();
    uint256 tokenLength = _tokens.length;

    if (tokenLength > _assetLimit)
      revert ErrorLibrary.TokenCountOutOfLimit(_assetLimit);

    for (uint256 i; i < tokenLength; i++) {
      address token = _tokens[i];
      _beforeInitCheck(token);
      if (_previousToken[token]) {
        revert ErrorLibrary.TokenAlreadyExist();
      }
      _previousToken[token] = true;
    }
    _resetPreviousTokenList(_tokens);
    tokens = _tokens;
+_currentSnapshotId++;
  }

similarly for initToken()

  function initToken(address[] calldata _tokens) external onlySuperAdmin {
    uint256 _assetLimit = protocolConfig().assetLimit();
    uint256 tokensLength = _tokens.length;
    if (tokensLength > _assetLimit)
      revert ErrorLibrary.TokenCountOutOfLimit(_assetLimit);
    if (tokens.length != 0) {
      revert ErrorLibrary.AlreadyInitialized();
    }
    for (uint256 i; i < tokensLength; i++) {
      address token = _tokens[i];
      _beforeInitCheck(token);
      if (_previousToken[token]) {
        revert ErrorLibrary.TokenAlreadyExist();
      }
      _previousToken[token] = true;
      tokens.push(token);
    }
    _resetPreviousTokenList(_tokens);
    emit PublicSwapEnabled(address(this));
+_currentSnapshotId++;
  }

Attack Scenario\ It is just the logic flaw, where the business function of vaultConfig's _currentSnapshotId is affected, as a result the protocol can't track the token update versioning.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

langnavina97 commented 3 months ago

Thank you for submitting the issue. We've resolved it and pushed the changes, which can be found here: https://github.com/Velvet-Capital/velvet-core/commit/25ef3e7a2bdf56f57901dba2f84ebf5ed5df7835

@burhankhaja