Summary
All used and donated tokens in the staking pool are lost to the last reward receiver due to an issue in the logic that distributes fees. The current code erroneously transfers the entire balance of the contract to the last receiver, rather than only the remaining portion of the fees. This results in the last receiver receiving more tokens than they are entitled to.
Vulnerability Details
The vulnerability arises from the fee distribution and how the contract handles the final balance transfer to the last reward receiver. Here's the critical portion of the code that leads to the problem:
* @notice Distributes rewards/fees based on balance changes in strategies since the last update
* @param _strategyIdxs indexes of strategies to update rewards for
* @param _data update data passed to each strategy
**/
function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
int256 totalRewards;
uint256 totalFeeAmounts;
uint256 totalFeeCount;
address[][] memory receivers = new address[][]();
uint256[][] memory feeAmounts = new uint256[][]();
for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
(
int256 depositChange,
address[] memory strategyReceivers,
uint256[] memory strategyFeeAmounts
@audit>>1. >> ) = strategy.updateDeposits(_data);
calls to update strategy rewards, _updateStrategyRewards, Staking pool will recive token from each strategy vault controller
function updateDeposits(
bytes calldata
)
external
virtual
onlyStakingPool
returns (int256 depositChange, address[] memory receivers, uint256[] memory amounts)
{
uint256 balance = token.balanceOf(address(this));
if (balance != 0) {
@audit>>2. >> token.safeTransfer(address(stakingPool), balance);
newTotalDeposits -= balance;
}
totalDeposits = newTotalDeposits;
}
Operator VCS
* @notice Updates deposit accounting and calculates fees on newly earned rewards
* @param _data encoded minRewards (uint256) - min amount of rewards required to claim (set 0 to skip reward claiming)
* @return depositChange change in deposits since last update
* @return receivers list of fee receivers
* @return amounts list of fee amounts
*/
function updateDeposits(
bytes calldata _data
)
external
override
onlyStakingPool
returns (int256 depositChange, address[] memory receivers, uint256[] memory amounts)
{
@audit>>3. >> if (balance != 0) {
@audit>>3. >> token.safeTransfer(address(stakingPool), balance);
newTotalDeposits -= balance;
}
totalDeposits = newTotalDeposits;
totalPrincipalDeposits = newTotalPrincipalDeposits;
}
The contract distributes fees to multiple receivers. Due to rounding down during fee calculations, there is usually a small remainder left after distributing the exact calculated amounts. To handle this remainder, the logic attempts to send the remaining balance to the last receiver. However, the following code:
* @notice Distributes rewards/fees based on balance changes in strategies since the last update
* @param _strategyIdxs indexes of strategies to update rewards for
* @param _data update data passed to each strategy
**/
function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
int256 totalRewards;
uint256 totalFeeAmounts;
uint256 totalFeeCount;
address[][] memory receivers = new address[][]();
}
if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
_mintShares(address(this), sharesToMint);
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
@audit>>4. >>>> if (feesPaidCount == totalFeeCount - 1) {
transferAndCallFrom(
address(this),
receivers[i][j],
@audit>>all funds >>>> balanceOf(address(this)),
"0x"
);
} else {
transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
}
emit UpdateStrategyRewards(msg.sender, totalStaked, totalRewards, totalFeeAmounts);
}
transfers the entire balance of the contract to the last receiver, including tokens that were meant for other participants or deposited tokens. As a result, all remaining funds in the staking pool contract are incorrectly sent to the last receiver.
### Root Cause
- **Precision Loss**: Fee amounts are rounded down, creating a small difference between the expected total fee and the remaining balance.
- **Incorrect Balance Transfer**: Instead of calculating the difference between the expected final fee and the actual balance, the logic transfers the entire contract balance, including deposited or donated tokens that should remain in the staking pool.
Impact
This bug leads to the following problems:
- **Loss of Funds**: All unused or donated tokens in the staking pool are transferred to the last reward receiver, effectively allowing them to receive all the remaining funds in the contract.
- **Incorrect Fee Distribution**: The last receiver receives more tokens than they are entitled to, while others may receive less or none at all.
Tools Used
Manual Code Review
Recommendations
1. **Fix the Final Balance Transfer Logic**: Instead of transferring the entire balance of the contract to the last receiver, calculate the correct difference between the remaining Reward balance and the fees paid so far. Only transfer the remainder to the last receiver.