In the LibSilo contract, the handleRainAndSops function contains a vulnerability where users who interact with the system frequently may inadvertently miss out on Season of Plenty (SOP) rewards. This occurs when a user's last update timestamp is after the most recent SOP event, causing the function to skip the reward distribution for that user. This bug can lead to significant loss of expected yields for active users.
Vulnerability Details
function handleRainAndSops(address account, uint32 lastUpdate, uint128 firstGerminatingRoots) private {
AppStorage storage s = LibAppStorage.diamondStorage();
if (s.season.lastSopSeason > lastUpdate) {
s.a[account].sop.plenty = balanceOfPlenty(account);
s.a[account].lastSop = s.season.lastSop;
}
}
The bug occurs because:
The function only updates SOP rewards if the lastSopSeason is greater than the user's lastUpdate.
If a user interacts with the system after an SOP event but before this function is called, their lastUpdate will be set to a value greater than lastSopSeason.
In subsequent calls, the condition s.season.lastSopSeason > lastUpdate will be false, causing the function to skip SOP reward distribution for this user.
Consider this scenario:
Initial state:
Current season: 100
Last SOP season: 95
User A's lastUpdate: 90
User B's lastUpdate: 90
SOP reward: 100 tokens per eligible user
Sequence of events:
Season 101: SOP occurs (s.season.lastSopSeason = 101)
Season 102: User A interacts with the system, lastUpdate set to 102
Season 103: handleRainAndSops is called for both users
Results:
User B:
Condition 101 > 90 is true
Receives 100 tokens SOP reward
lastSop updated to 101
User A:
Condition 101 > 102 is false
Receives 0 tokens SOP reward
lastSop remains unchanged
In this scenario, User A misses out on 100 tokens of rewards simply due to the timing of their interaction with the system.
Impact
Theft of unclaimed yield
This bug disproportionately affects active users and can lead to significant loss of expected yields. It undermines the fairness of the reward distribution system and may discourage frequent interaction with the protocol.
Proof of concept
BIC Response
Upon review, we have found that this is not a valid bug. The report is based on the following assumption:
If a user interacts with the system after an SOP event but before this function is called
This scenario does not occur, because in said interaction (aka a mow), the logic handleRainAndSops is executed because s.season.lastSopSeason > lastUpdate is true (because this is the first update of the season), and thus credits the Plenty to the user.
Further, we have executed your provided POC on the current master branch and the user gains Plenty as expected.
Therefore, we are closing this report and no reward will be issued.
function test_missedSopRewards() public {
address user1 = address(1);
address user2 = address(2);
uint256 initialDeposit = 1000e6;
uint256 sopReward = 100e6;
vm.startPrank(user1);
bs.deposit(C.BEAN, initialDeposit, INTERNAL);
vm.stopPrank();
vm.startPrank(user2);
bs.deposit(C.BEAN, initialDeposit, INTERNAL);
vm.stopPrank();
for (uint i = 0; i < 100; i++) {
season.rainSunrise();
}
setReserves(C.BEAN_ETH_WELL, 1000000e6, 1100e18);
season.rainSunrise(); // Now at season 101, SOP occurred
vm.prank(user1);
bs.mow(user1, C.BEAN);
season.rainSunrise(); // Now at season 102
bs.mow(user1, C.BEAN);
bs.mow(user2, C.BEAN);
uint256 user1Plenty = bs.balanceOfPlenty(user1, C.BEAN_ETH_WELL);
uint256 user2Plenty = bs.balanceOfPlenty(user2, C.BEAN_ETH_WELL);
assertEq(user1Plenty, 0, "User1 should not have received SOP rewards");
assertEq(user2Plenty, sopReward, "User2 should have received SOP rewards");
}