sherlock-audit / 2024-06-leveraged-vaults-judging

9 stars 8 forks source link

zhuying - Don't check the emission is over when setting new emission rate #111

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

zhuying

Medium

Don't check the emission is over when setting new emission rate

Summary

Don't check the emission is over when setting new emission rate.

Vulnerability Detail

        if (index < settings.numRewardTokens) {
            // Safety check to ensure that the correct token is specified, we can never change the
            // token address once set.
            require(state.rewardToken == rewardToken);
            // Modifies the emission rate on an existing token, direct claims of the token will
            // not be affected by the emission rate.
            // First accumulate under the old regime up to the current time. Even if the previous
            // emissionRatePerYear is zero this will still set the lastAccumulatedTime to the current
            // blockTime.
            _accumulateSecondaryRewardViaEmissionRate(index, state, totalVaultSharesBefore);

            // Save the new emission rates
            state.emissionRatePerYear = emissionRatePerYear;
            if (state.emissionRatePerYear == 0) {
                state.endTime = 0;
            } else {
                require(block.timestamp < endTime);
                state.endTime = endTime;
            }
            VaultStorage.getVaultRewardState()[index] = state;

When we want to set new emissionRatePerYear or set new endTime, it didn't check the old endTime is past or not. If the old endTime is not past and new emissionRatePerYear is set, the account will get wrong rewards.

Impact

The account's rewards is distributed wrongly in some case.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/main/leveraged-vaults-private/contracts/vaults/common/VaultRewarderLib.sol#L86-L106

Tool used

manual

Recommendation

+          require(block.timestamp > state.endTime);
            if (state.emissionRatePerYear == 0) {
                state.endTime = 0;
            } else {
                require(block.timestamp < endTime);
                state.endTime = endTime;
            }
sherlock-admin3 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Low/QA at most on admin controlled call