User can avoid losing his Guild tokens by moving them out of the gauge before notifyLoss is called

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L123-L129
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L212-L217

Vulnerability details

Impact

Since all operations are performed on the blockchain savvy users can move funds from a soon-to-be-slashed gauge before notifyLoss is called, as a result users are able to put their Guild tokens into riskier gauges without suffering the consequences.

Proof of Concept

If the LendingTerm has a bad debt, it will be auctioned, after auction is completed we cover our losses and call notifyLoss on the affected gauge/term
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L789-L798
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L305

    function notifyPnL(
        address gauge,
        int256 amount
    ) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
        uint256 _surplusBuffer = surplusBuffer;
        uint256 _termSurplusBuffer = termSurplusBuffer[gauge];
        address _credit = credit;

        // handling loss
        if (amount < 0) {
            uint256 loss = uint256(-amount);

            // save gauge loss
            GuildToken(guild).notifyGaugeLoss(gauge);

preventing users from withdrawing their funds out of it without first burning their Guild tokens.
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L123
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L212-L217

    function _decrementGaugeWeight(
        address user,
        address gauge,
        uint256 weight
    ) internal override {
        uint256 _lastGaugeLoss = lastGaugeLoss[gauge];
        uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][user];
        require(
            _lastGaugeLossApplied >= _lastGaugeLoss,
            "GuildToken: pending loss"
        );

Since this is a lengthy process, users can move tokens out of the gauge long before the auction is over, moreover, it is possible to frontrun the AuctionHouse.bid transaction and withdraw tokens at the last moment.

Test case for AuctionHouse.t.sol, add a repayment delay
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/test/unit/loan/AuctionHouse.t.sol#L80

+ maxDelayBetweenPartialRepay: 86400 * 30,

    function testFrontrunBid() public {
        // prepare
        uint256 borrowAmount = 20_000e18;
        uint256 collateralAmount = 15e18;
        collateral.mint(address(borrower), collateralAmount);
    
        // borrow
        vm.startPrank(borrower);
        collateral.approve(address(term), collateralAmount);
        bytes32 loanId = term.borrow(borrowAmount, collateralAmount);
        vm.stopPrank();

        // 1 year later, interest accrued
        vm.warp(block.timestamp + term.YEAR());
        vm.roll(block.number + 1);

        // call
        vm.startPrank(caller);
        term.call(loanId);
        vm.stopPrank();

        uint256 PHASE_1_DURATION = auctionHouse.midPoint();
        uint256 PHASE_2_DURATION = auctionHouse.auctionDuration() - auctionHouse.midPoint();
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + PHASE_1_DURATION + PHASE_2_DURATION / 2);

        // frontrun bidder
        guild.decrementGauge(address(term), 1e17);//another bug prevents us from withdrawing all tokens tho
        // bid
        credit.mint(bidder, 11_000e18);
        vm.startPrank(bidder);
        credit.approve(address(term), 11_000e18);
        auctionHouse.bid(loanId);
        vm.stopPrank();
    }

Tools Used

Foundry

Recommended Mitigation Steps

Perhaps we should freeze all weight operations during the auction, however users can still frontrun AuctionHouse.startAuction

Assessed type

MEV

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #906

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #877

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid