A malicious owner can steal all unclaimed rewards and break the reward accounting mechanism
Even if the owner is a good guy but the fact that there exists a rug vector available may negatively impact the protocolโs reputation. Or maybe the hotkeys of a multi-sig wallet may be stolen. Furthermore, since this contract is meant to be used by other projects, the trustworthiness of every project cannot be vouched for.
The problem lies in the fact that the flywheelRewards is not immutable. Letโs check a hypothetical process a malicious owner can take.
The boostedBalanceOf() function is used to calculate the boosted balance of a user in a given strategy. By creating and setting a booster contract that returns zero when users call boostedBalanceOf() in a situation where the user address is not under the attackerโs control, and returning arbitrary values for those under his/her control, an attacker can choose specific amounts of rewardToken to assign to himself/herself. The attacker can then call claimRewards() to withdraw the funds. Any amounts that the attacker assigns to himself/herself over the amount that normally would have been assigned, upon claiming, is taken from other usersโ unclaimed balances since tokens are custodied by the flywheelRewards address rather than per-user accounts.
function setBooster(IFlywheelBooster newBooster) external onlyOwner {
flywheelBooster = newBooster;
emit FlywheelBoosterUpdate(address(newBooster));
}
In the function accrueUser() we have the supplierToken variable defined in a way that calls boostedBalanceOf():
function accrueUser(ERC20 strategy, address user, uint256 index) private returns (uint256) {
// load indices
uint256 supplierIndex = userIndex[strategy][user];
// sync user index to global
userIndex[strategy][user] = index;
// if user hasn't yet accrued rewards, grant them interest from the strategy beginning if they have a balance
// zero balances will have no effect other than syncing to global index
if (supplierIndex == 0) {
supplierIndex = ONE;
}
uint256 deltaIndex = index - supplierIndex;
// use the booster or token balance to calculate reward balance multiplier
uint256 supplierTokens = address(flywheelBooster) != address(0)
? flywheelBooster.boostedBalanceOf(strategy, user)
: strategy.balanceOf(user);
// accumulate rewards by multiplying user tokens by rewardsPerToken index and adding on unclaimed
uint256 supplierDelta = (supplierTokens * deltaIndex) / ONE;
uint256 supplierAccrued = rewardsAccrued[user] + supplierDelta;
rewardsAccrued[user] = supplierAccrued;
emit AccrueRewards(strategy, user, supplierDelta, index);
return supplierAccrued;
}
Finally, the function claimRewards() is defined in the preceding way:
function claimRewards(address user) external {
uint256 accrued = rewardsAccrued[user];
if (accrued != 0) {
rewardsAccrued[user] = 0;
rewardToken.safeTransferFrom(address(flywheelRewards), user, accrued);
emit ClaimRewards(user, accrued);
}
}
Manual Review
<https://twitter.com/RugDocIO/status/1411732108029181960>
<https://twitter.com/Mudit__Gupta/status/1675584195798913024>
Make flywheelRewards address immutable, or only allow it to change if there are no current users
Decimal
The text was updated successfully, but these errors were encountered:
All reactions