DeFiHardhat
21,000 USDC
View results
Submission Details
Severity: high
Invalid

`LibSilo.sol::_mow` potential for skipped updates

Summary

The concern about "Potential for Skipped Updates" in the __mow function from the LibSilo.sol file revolves around the scenario where updates to the lastStem value might be performed without corresponding actions that should logically accompany such updates, particularly when the balance-derived value is zero.

Vulnerability Details

`function _mow(address account, address token) internal {
    AppStorage storage s = LibAppStorage.diamondStorage();
    
    // if the user has not migrated from siloV2, revert.
    (bool needsMigration, uint32 lastUpdate) = migrationNeeded(account);
    require(!needsMigration, "Silo: Migration needed");


    // if the user hasn't updated prior to the seedGauge/siloV3.1 update,
    // perform a one time `lastStem` scale.
    if (
        (lastUpdate < s.season.stemScaleSeason && lastUpdate > 0) || 
        (lastUpdate == s.season.stemScaleSeason && checkStemEdgeCase(account))
    ) {
        migrateStems(account);
    }


    // sop data only needs to be updated once per season,
    // if it started raining and it's still raining, or there was a sop
    uint32 currentSeason = s.season.current;
    if (s.season.rainStart > s.season.stemStartSeason) {
        if (lastUpdate <= s.season.rainStart && lastUpdate <= currentSeason) {
            // Increments `plenty` for `account` if a Flood has occured.
            // Saves Rain Roots for `account` if it is Raining.
            handleRainAndSops(account, lastUpdate);
        }
    }
    
    // End account germination.
    if (lastUpdate < currentSeason) {
        LibGerminate.endAccountGermination(account, lastUpdate, currentSeason);
    }
    // Calculate the amount of Grown Stalk claimable by `account`.
    // Increase the account's balance of Stalk and Roots.
    __mow(account, token);


    // update lastUpdate for sop and germination calculations.
    s.a[account].lastUpdate = currentSeason;
}

`
Consider a scenario where an account has a _bdv of zero for a particular token. This might occur if the account has previously mowed all their stalk and currently holds no balance-derived value that would result in new stalk generation. Here's what happens:

  1. Initial State:
    _lastStem: 100
    _stemTip: 150 (indicating that the stem has grown since the last update)
    _bdv: 0 (the account has no balance-derived value)

  2. Function Execution:
    The function checks if _bdv > 0 and _lastStem != _stemTip. Since _bdv is 0, the condition fails.
    No mowing occurs because there's no balance to derive new stalk from.
    However, the function still updates _lastStem to _stemTip (150).

  3. Result:
    The lastStem is updated to 150, even though no actual mowing action took place.
    If, in the future, the account acquires a non-zero _bdv and the stem tip has not advanced further, the function will not compute any mowing rewards for the growth from 100 to 150, because the system now incorrectly assumes that this growth has already been accounted for.

Impact

If _bdv is zero and _lastStem does not equal _stemTip, the function will not perform any operations but will still update lastStem. This could potentially skip necessary updates or checks that should have been performed.

Tools Used

Manual Review

Recommendations

To address this, the function could be modified to update _lastStem only when mowing actions actually occur, or additional logic could be added to handle cases where _bdv is zero differently, ensuring that future calculations correctly account for all periods of stem growth:

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.