katta_seller - Reward calculation is broken

[sherlock-admin4]

katta_seller

High

Reward calculation is broken

Summary

VaultRewarderLib is used to update accumulatedRewardPerVaultShare by accumulating any rewards based on the emission rate, the current total Vault shares, and the time interval between the current block time and the last lastAccumulatedTime.

Vulnerability Detail

The function _getAccumulatedRewardViaEmissionRate calculates additionalIncentiveAccumulatedPerVaultShare. The calculation is as follows:

    function _getAccumulatedRewardViaEmissionRate(
        VaultRewardState memory state,
        uint256 totalVaultSharesBefore,
        uint256 blockTime
    ) private pure returns (uint256) {
        // Short circuit the method with no emission rate
        if (state.emissionRatePerYear == 0) return state.accumulatedRewardPerVaultShare;
        require(0 < state.endTime);
        uint256 time = blockTime < state.endTime ? blockTime : state.endTime;

        uint256 additionalIncentiveAccumulatedPerVaultShare;
        if (state.lastAccumulatedTime < time && 0 < totalVaultSharesBefore) {
            // NOTE: no underflow, checked in if statement
            uint256 timeSinceLastAccumulation = time - state.lastAccumulatedTime;
            // Precision here is:
            //  timeSinceLastAccumulation (SECONDS)
            //  emissionRatePerYear (REWARD_TOKEN_PRECISION)
            //  INTERNAL_TOKEN_PRECISION (1e8)
            // DIVIDE BY
            //  YEAR (SECONDS)
            //  INTERNAL_TOKEN_PRECISION (1e8)
            // => Precision = REWARD_TOKEN_PRECISION * INTERNAL_TOKEN_PRECISION / INTERNAL_TOKEN_PRECISION
            // => rewardTokenPrecision
            additionalIncentiveAccumulatedPerVaultShare =
                (timeSinceLastAccumulation
                    * uint256(Constants.INTERNAL_TOKEN_PRECISION)
                    * state.emissionRatePerYear)
                / (Constants.YEAR * totalVaultSharesBefore);   
                //@audit-issue can be rounded down to zero if reward token is in low decimals

        }

        return state.accumulatedRewardPerVaultShare + additionalIncentiveAccumulatedPerVaultShare;
    }

The function _getAccumulatedRewardViaEmissionRate calculates additionalIncentiveAccumulatedPerVaultShare. The calculation is as follows:

additionalIncentiveAccumulatedPerVaultShare  = (timeSinceLastAccumulation * 1e8 * emissionRate) / (31536000 * totalVaultSharesBefore)

Since totalVaultSharesBefore has 8 decimals and emissionRate is in the reward token's decimal precision, if the reward token has fewer decimals (e.g., 8 decimals), an attacker can exploit this by calling claimAccountRewards in every block to make additionalIncentiveAccumulatedPerVaultShare to zero and update the lastAccumulatedTime .

Impact

The calculation of additionalIncentiveAccumulatedPerVaultShare can be rounded down to zero if the reward token has low decimals. This effectively prevents the accumulation of rewards.

Code Snippet

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

Tool used

Manual Review

Recommendation

Use higher precision to avoid rounding down to zero in the reward calculation

[sherlock-admin3]

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

0xmystery commented:

Low/QA at most due to dust amount loss

[notN000B]

@mystery0x @jeffywu

This is not dust amount loss. It prevents the accumulation of rewards. Can someone escalate this for me?

This is my first contest in Sherlock so i can't:(

[jeffywu]

My reading of this is some sort of DoS attack where the accumulation is always rounded down. However, it requires the attacker to constantly execute the claim function and pay gas costs and it's not clear what sort of gain the attacker would get.

The gas costs would significantly outweigh any benefit to the attacker. To me, the non-reward label appears valid.

[WangSecurity]

61 which is the same issue, will be validated, but this issue is not a sufficient duplicate based on the following requirements for duplication:

Identify a valid attack path or vulnerability path Fulfills other submission quality requirements (e.g. provides a PoC for categories that require one)

And POC requirements:

PoC is required for all issues falling into any of the following groups: issues related to precision loss

[notN000B]

@WangSecurity

Identify a valid attack path or vulnerability path Fulfills other submission quality requirements (e.g. provides a PoC for categories that require one)

Submission includes valid attack path and POC both.

Since totalVaultSharesBefore has 8 decimals and emissionRate is in the reward token's decimal precision, if the reward token has fewer decimals (e.g., 8 decimals), an attacker can exploit this by calling claimAccountRewards in every block to make additionalIncentiveAccumulatedPerVaultShare to zero and update the lastAccumulatedTime .

The above lines contain a valid attack path (i.e., calling claimAccountRewards in every block) and detail why this rounds down to zero, which serves as a valid PoC.

My reading of this is some sort of DoS attack where the accumulation is always rounded down.

This sponsor comment indicates that the submission sufficiently explains the issue. Every necessary detail for an ideal submission is present here. Even a newbie SR can easily understand the issue

1. The issue explains the core problem and how rounding to zero can be performed for reward tokens with fewer decimals
2. It explains the impact, which is preventing the accumulation of rewards

what else do I need to include in the report?

[notN000B]

@jeffywu @xiaoming9090 @mystery0x

This is my first contest, and it seems you are important guys of the contest. Could you please provide your point of view regarding the sufficiency of the proof in this issue ?

[WangSecurity]

The finding lacks sufficient POC for precision loss. It doesn't show that precision loss may indeed happen here and has to have a numerical example of values that would cause the precision loss here.

I see that it's your first contest, excuse me for rules being not clear in this case, but your report lacks sufficient proof that precision loss actually happens here.

[notN000B]

@WangSecurity

The finding lacks sufficient POC for precision loss

I respectfully disagree with your comment. There is enough evidence to validate that the division will round down to zero. The sponsor understood the issue before seeing any duplicates.

Since totalVaultSharesBefore has 8 decimals and emissionRate is in the reward token's decimal precision, if the reward token has fewer decimals (e.g., 8 decimals), an attacker can exploit this by calling claimAccountRewards in every block to make additionalIncentiveAccumulatedPerVaultShare to zero and update the lastAccumulatedTime .

These lines clearly explain that since the emission rate has fewer decimals than denominator, calling _getAccumulatedRewardViaEmissionRate in every block will lead to zero reward accumulation..

I'm surprised to see that A/B, where B is a token amount in 8 decimals and A is a token amount in less than 8 decimals, requires a numerical example to understand the rounding behavior. Any judge,dev or newbie SR would understand that A/B will round down to zero in such cases. I have documented that calling this function in every block will lead to zero rewards since the emission rate is in reward decimals, which could be less than or equal to 8 decimals

I believe this issue is valid. A submission should have a valid POC, whether it is coded, documented verbally, or shown numerically, and this has it

I can link multiple examples where 2 liner report is validated by you.

1.Issue 162 heck this report quality by the lead auditor of the contest. Even you requested a POC and then accepted it.

2. issue 80 Requested a POC then accepted it.

In these cases you requested poc just to understand the issue while in this case you can clearly the issue.

@mystery0x @xiaoming9090 I guess you are lead Watson and lead judge in this contest so please respond to this issue. I don't know where to raise my concern

[WangSecurity]

The decision remains the same, for precision loss the report has to have a numerical example (ideally coded) as a POC to be sufficient. This report doesn’t, it just says there will be a precision loss if reward token has 8 decimals.