Description

Each staking manager has an associated token for accounting purposes. When a user accrues rewards, the corresponding tokens are minted. When a user withdraws tokens, the contract uses the sum of their deposits and their reward token balance. From the withdraw function,

// Withdraw from definitive vault and include reward token balances
uint256 underlyingAmount = withdrawFromDefinitive(
  _index,
  _lpTokenAmount + rewardTokenBalance
);

emit Withdraw(msg.sender, underlyingAmount);

// Transfer to user
IERC20 underlying = IERC20(underlyingTokenAddresses[_index]);
underlying.approve(msg.sender, underlyingAmount);
underlying.transfer(msg.sender, underlyingAmount);

The reward token is deployed separately by the owner, who uses the admin role to grant the corresponding staking manager the ability to mint and burn tokens.

Impact

This means that the owner retains the ability to arbitrarily mint and burn tokens. By granting the MINTER_ROLE to an account they control, the owner can

1. decrease the shares of other users and

2. increase their own shares.

At any time, the owner can mint a large amount of tokens for themselves and withdraw the entire lpTokensStaked. As a consequence, in the event of a key compromise, all users would be exposed to potential loss of funds. Additionally, this requires unnecessary trust in the owner, which might discourage use of the protocol.

Recommendations

We recommend deploying the token contract from the staking manager constructor and removing the owner's responsibility to grant roles. Alternatively, the ownership of the contract could be transferred to the staking manager.

Remediation

In commit , Rainmaker fixed this issue by deploying the token contract directly from the staking manager constructor.