sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Ch_301 - users will steal/lose some CRV rewards from `WCurveGauge.sol` #140

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Ch_301

high

users will steal/lose some CRV rewards from WCurveGauge.sol

Summary

from Curve Docs

The weight system
Each gauge also has a weight and a type. Those weights represent how much of the daily CRV inflation will be received by the liquidity gauge.

WCurveGauge.sol have this logic to mint CRV reward for curve gauge and update the tracking system for reward

    function _mintCrv(ILiquidityGauge gauge, uint256 gid) internal {
        uint256 balanceBefore = CRV.balanceOf(address(this));
        ILiquidityGaugeMinter(gauge.minter()).mint(address(gauge));
        uint256 balanceAfter = CRV.balanceOf(address(this));
        uint256 gain = balanceAfter - balanceBefore;
        uint256 supply = gauge.balanceOf(address(this));
        if (gain > 0 && supply > 0) {
            accCrvPerShares[gid] += (gain * 1e18) / supply;
        }
    }

Vulnerability Detail

On this POC I will take as an assumption the gauge has a constant weight to simplify the computation. This is the constant weight: If you deposit in the gaugeX 10 LP tokens you will receive 2 CRV every week (so if you deposit 20 LP tokens you will receive 4 CRV every week).

THE TIMELINE STARTS HERE Day_one:

After_One_Week:

After_One_Week:

After_One_Week: if any user decides to close his position we should have these results

THE END OF THE TIMELINE

Let's take user_01 as an example: (POC) user_01 invoke burn() this logic will calculate the rewards

        uint256 stCrv = (stCrvPerShare * amount) / 1e18;
        uint256 enCrv = (accCrvPerShares[gid] * amount) / 1e18;
        if (enCrv > stCrv) {
            rewards = enCrv - stCrv;
            CRV.safeTransfer(msg.sender, rewards);
        }

we have: (ignore 1e18 ) stCrv = 0 10 = 0 enCrv = 2.25 10 = 22.5 (we only have 16 CRV in this contract) so enCrv > stCrv ==> 22.5 > 0 == true rewards = enCrv - stCrv ==> 22.5 - 0 = 22.5 so rewards is 22.5 CRV this amount is not even here

You can do different scenarios with all the users and some of them end up stealing rewards (e.g:user_03 will receive 5 CRV so he gets an extra 3 CRV)

Impact

users will steal/lose some rewards from WCurveGauge.sol

Code Snippet

Tool used

Manual Review

Recommendation

Update this part

        if (gain > 0 && supply > 0) {
            accCrvPerShares[gid] += (gain * 1e18) / supply;
        }

from _mintCrv()

Ch-301 commented 1 year ago

Escalate for 10 USDC

The POC above shows exactly how this part of the logic

        if (gain > 0 && supply > 0) {
            accCrvPerShares[gid] += (gain * 1e18) / supply;
        }

failed to calculate the correct value of accCrvPerShares[gid]

sherlock-admin commented 1 year ago

Escalate for 10 USDC

The POC above shows exactly how this part of the logic

        if (gain > 0 && supply > 0) {
            accCrvPerShares[gid] += (gain * 1e18) / supply;
        }

failed to calculate the correct value of accCrvPerShares[gid]

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

hrishibhat commented 1 year ago

Escalation rejected

Invalid issue The example above uses the total supply of CRV but the code is using the total supply of the LP deposited into the gauge, not CRV.

sherlock-admin commented 1 year ago

Escalation rejected

Invalid issue The example above uses the total supply of CRV but the code is using the total supply of the LP deposited into the gauge, not CRV.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.