Summary
Rewards distribution flow can be blocked by fee receivers through token transfer callback
Vulnerability Details
The internal function StakingPool#_updateStrategyRewards() is expected to update and distribute rewards if net positive earned from strategies. Fees/rewards are minted as shares and sent to receivers using function ERC677#transferAndCallFrom(), which has contract callback if receiver is a contract.
function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
...
...
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
if (feesPaidCount == totalFeeCount - 1) {
@> transferAndCallFrom(
address(this),
receivers[i][j],
balanceOf(address(this)),
"0x"
);
} else {
@> transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
}
emit UpdateStrategyRewards(msg.sender, totalStaked, totalRewards, totalFeeAmounts);
}
The function StakingPool#_updateStrategyRewards() is called from public functions StakingPool#updateStrategyRewards() and StakingPool#removeStrategy()
function removeStrategy(
uint256 _index,
bytes memory _strategyUpdateData,
bytes calldata _strategyWithdrawalData
) external onlyOwner {
require(_index < strategies.length, "Strategy does not exist");
uint256[] memory idxs = new uint256[]();
idxs[0] = _index;
@> _updateStrategyRewards(idxs, _strategyUpdateData);
IStrategy strategy = IStrategy(strategies[_index]);
uint256 totalStrategyDeposits = strategy.getTotalDeposits();
if (totalStrategyDeposits > 0) {
strategy.withdraw(totalStrategyDeposits, _strategyWithdrawalData);
}
for (uint256 i = _index; i < strategies.length - 1; i++) {
strategies[i] = strategies[i + 1];
}
strategies.pop();
token.safeApprove(address(strategy), 0);
}
function updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) external {
if (msg.sender != rebaseController && !_strategyExists(msg.sender))
revert SenderNotAuthorized();
@> _updateStrategyRewards(_strategyIdxs, _data);
}
The function StakingPool#updateStrategyRewards() can be called from rebaseController or, called from strategy contracts. In strategy contracts, there are 2 flows that call the function: addFee and updateFee:
function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
@> _updateStrategyRewards();
fees.push(Fee(_receiver, _feeBasisPoints));
if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge();
}
function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
@> _updateStrategyRewards();
if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge();
}
function _updateStrategyRewards() internal {
address[] memory strategies = stakingPool.getStrategies();
uint256[] memory strategyIdxs = new uint256[]();
for (uint256 i = 0; i < strategies.length; ++i) {
strategyIdxs[i] = i;
}
@> stakingPool.updateStrategyRewards(strategyIdxs, "");
}
So, the with the single failure point here at the function _updateStrategyRewards, malicious fee receivers can conditionally block 3 flows: removing strategies, adding fees in strategies, updating fees in strategies
Impact
A malicious fee receiver can conditionally block 3 flows: removing strategies, adding fees in strategies, updating fees in strategies
Tools Used
Manual
Recommendations
Consider adding mechanism to try/catch to skip revert callbacks, or force transfers
