Assessment reports>Rainmaker>Medium findings>Excessive owner responsibility creates deployment risks
Category: Code Maturity

Excessive owner responsibility creates deployment risks

Medium Severity
Medium Impact
Low Likelihood

Description

Each staking manager, from construction, holds an array of underlying token addresses:

// Constructor
constructor(
  address[] memory _underlyingAddresses,
  address _rewardTokenAddress,
  address _definitiveVaultAddress
) {
  underlyingTokenAddresses = _underlyingAddresses;
  rewardToken = DefinitiveRewardToken(_rewardTokenAddress);
  definitiveVault = IDefinitiveVault(_definitiveVaultAddress);
}

The precise order and contents of this array are extremely important because in depositIntoDefinitive, the amounts array must correspond exactly to both underlyingTokenAddresses and the token addresses in the vault.

/**
 * @dev Deposit tokens into Definitive vault end-to-end (deposit + enter)
 * @return Staked amount (lpTokens)
 */
function depositIntoDefinitive(
  uint256 _underlyingAmount,
  uint8 _index
) private returns (uint256) {
  IERC20 underlying = IERC20(underlyingTokenAddresses[_index]);

  uint256[] memory amounts = new uint256[](underlyingTokenAddresses.length);
  amounts[_index] = _underlyingAmount;

  // 1. Approve vault to spend underlying
  underlying.approve(address(definitiveVault), _underlyingAmount);

  // 2. Deposit into the vault
  definitiveVault.deposit(amounts, underlyingTokenAddresses);

  // 3. Enter into the strategy using 0 as minAmountsOut to get standard slippage
  return definitiveVault.enter(amounts, 0);
}

This means that during deployment, the owner is responsible for ensuring that _underlyingAddresses matches the vault's LP_UNDERLYING_TOKENS.

Additionally, the owner needs to grant the staking manager the MINTER_ROLE in its corresponding reward token.

Impact

If the underlyingTokenAddresses array does not match LP_UNDERLYING_TOKENS, the protocol may experience incorrect accounting or broken functionality.

If the staking manager is not granted the required role, then deposits and withdrawals would eventually fail. Worse, if MINTER_ROLE on one token is mistakenly granted to multiple different staking managers, they could experience severe accounting issues and users may lose funds.

Recommendations

We recommend that the protocol determine the underlying token addresses from the given vault as a single source of truth. The second issue is mitigated by the recommendation in .

Remediation

Rainmaker fixed these risks in and the remediations for .

Zellic © 2024Back to top ↑