Description

The updateDeposits function is called by the staking pool to update the amount of accrued fees based on the profit and loss sustained by the vaults. It calls getDepositChange to calculate the difference between the current total balance across all the vaults and the totalDeposits, which is the baseline amount of assets that the vault is expected to have — any assets above that amount are profits that are taxed by the fees, and any assets below that amount are losses that do not incur fees.

The getDepositChange function takes into account the current balance of the contract as well as the balances of all of the vaults:

function getDepositChange() public view returns (int) {
    uint256 totalBalance = token.balanceOf(address(this));

    for (uint256 i = 0; i < vaults.length; ++i) {
        totalBalance += vaults[i].getTotalDeposits();
    }
    return int(totalBalance) - int(totalDeposits);
}

So, if any third party donates assets, either in the form of the underlying token sent to the PolygonStrategy (or any vault) or in the form of staked tokens sent to any vault, those assets will be accounted as profits and increase the amount of fees returned by updateDeposits.

The caller of updateDeposits is the _updateStrategyRewards function in the StakingPool contract in the core directory. In this contract, there is a safety check that skips sending any of the fees to anyone if the amount is unexpectedly high:

function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
    // [...]

    // sum up rewards and fees across strategies
    // [...]

    // update totalStaked if there was a net change in deposits
    // [...]

    // calulate fees if net positive rewards were earned
    // [...]

    // safety check
    if (totalFeeAmounts >= totalStaked) {
        totalFeeAmounts = 0;
    }

    // distribute fees to receivers if there are any
    if (totalFeeAmounts > 0) {
        uint256 sharesToMint = (totalFeeAmounts * totalShares) /
            (totalStaked - totalFeeAmounts);
        _mintShares(address(this), sharesToMint);

        // [...]
    }

    emit UpdateStrategyRewards(msg.sender, totalStaked, totalRewards, totalFeeAmounts);
}

It is important to note that the recipients of the fees are the designated receiver addresses of the fees array, which is only modifiable by the owner. These recipients are not necessarily the same parties as the depositors to the strategy or the validator MEV recipients.

Impact

This safety check may incentivize abnormal profit-seeking behavior, depending on the parameters and the ability for various parties to collude. For example, if the addresses in the fees array are completely separate from the depositors to the strategy, and the validator MEV recipient's share is zero or insignificant, then the depositors may be able to collude in order to artificially greatly inflate the profits seen by the strategy, to trigger this edge case. The extra value they donate will be entirely returned to them as profits, since the fees are reduced to zero.

Recommendations

Without analyzing the rest of the protocol and broader market, we have no specific recommendation on how this edge case may be prevented. We recommend designing the fee structure in a way that is safe against the profit-seeking collusion of any set of actors in this system, and that may require a safety check in this contract that sets aside excess profits or caps the fees to ensure that this core safety-check condition is never triggered.

Remediation

This issue was present and documented in a previous audit of this project, and it was acknowledged by Stake.link. It is written here for completeness only.