hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

Transferring ownership of a Portfolio is not updated (reflected) on `PortfolioInfolList` #83

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x750cf9deb0664a4b9ca89429d70e89fcb855cd87c54bc5f584574925854a97c4 Severity: low

Description: Description

There is a PortfolioInfolList contains PortfoliolInfo struct list as following:

  struct PortfoliolInfo {
    address portfolio;
    address tokenExclusionManager;
    address rebalancing;
    address owner;
    address assetManagementConfig;
    address feeModule;
    address vaultAddress;
    address gnosisModule;
  }

This list is set when _createPortfolio is called.

    PortfolioInfolList.push(
      PortfoliolInfo(
        address(portfolio),
        address(_tokenExclusionManager),
        address(rebalancing),
        msg.sender, // owner
        address(_assetManagementConfig),
        address(_feeModule),
        address(vaultAddress),
        address(module)
      )
    );

this portfolio info maybe intended for metadata-like information for public to know, via this function

  /**
   * @notice This function returns the Portfolio address at the given portfolio id
   * @param portfoliofundId Integral id of the portfolio fund whose Portfolio address is to be retrieved
   * @return Return the Portfolio address of the fund
   */
  function getPortfolioList(
    uint256 portfoliofundId
  ) external view virtual returns (address) {
    return address(PortfolioInfolList[portfoliofundId].portfolio);
  }

Among those addresses, there is only one which is not a proxy contract, the owner address.

This owner address is the one who create portfolio.

If we take a look at Portfolio contract, it's a type of Ownable contract, which have an owner, and transferOwnership function.

contract Portfolio is OwnableUpgradeable, UUPSUpgradeable, VaultManager {

Here is the case, when a portfolio is being transfered to other entity (EOA or Multisig), which is possible since it's an Ownable contract, currently there is no override implementation to capture this tranferOwnership and update the PortfolioInfolList

Meaning, if a portfolio being transfer to other entity, when getPortfolioList is called to fetch info about the portfolio, the owner is still the old one.

Impact

When portfolio is transfering its ownership to other entity (EOA or Multisig wallet), the getPortfolioList info about owner is still the old one.

Recommendation

Override the transferOwnership and update PortfolioInfolList owner of selected portfolio