Ethereum Credit Guild - SweetDream's results

Lines of code

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

Vulnerability details

Impact

Users will lost all staked tokens after pnl loss.

Proof of Concept

If a bidder bids on a loan between mid points and duration periods, the system incurs some losses and is called out notifyPnl(). In that case, the loss amount will be negative value.

    function notifyPnL(
        address gauge,
        int256 amount
    ) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
        ...
        if (amount < 0) {
            uint256 loss = uint256(-amount);

            // save gauge loss
            GuildToken(guild).notifyGaugeLoss(gauge);
            ...
        }
        else if (amount > 0) {
            ...
        }
        ...
    }

To save guage loss, GuildToken(guild).notifyGaugeLoss(gauge) is called in notifyPnl function.

    function notifyGaugeLoss(address gauge) external {
        require(msg.sender == profitManager, "UNAUTHORIZED");

        // save gauge loss
        lastGaugeLoss[gauge] = block.timestamp;
        emit GaugeLoss(gauge, block.timestamp);
    }

As you can see above, lastGaugeLoss[gauge] will be updated to block.timestamp.

In getRewards function, there is no initialization for the userStake.lastGaugeLoss before if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) part. So userStake.lastGaugeLoss is always set to 0, and slashed flag is set to true.

    function getRewards(
        address user,
        address term
    )
        public
        returns (
            uint256 lastGaugeLoss, // GuildToken.lastGaugeLoss(term)
            UserStake memory userStake, // stake state after execution of getRewards()
            bool slashed // true if the user has been slashed
        )
    {
        bool updateState;
        lastGaugeLoss = GuildToken(guild).lastGaugeLoss(term);
        if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) {
            slashed = true;
        }
        ...
    }

In unStake function, if slashed flag is true, the function returns after calling getRewards. So users cannot get their staked tokens.

    function unstake(address term, uint256 amount) external {
        // apply pending rewards
        (, UserStake memory userStake, bool slashed) = getRewards(
            msg.sender,
            term
        );

        console2.log("slashed", slashed);

        // if the user has been slashed, there is nothing to do
        if (slashed) return;
        ...
    }

Foundry Test code

    function testSlashFlag() public {
        address term1 = address(new MockLendingTerm(address(core)));
        guild.addGauge(1, term1);
        guild.mint(address(this), 100e18);
        guild.incrementGauge(term1, 100e18);

        bool slashed = false;

        address user1 = address(19028109281092);

        credit.mint(user1, 100e18);

        (, , slashed ) = sgm.getRewards(user1, term1);

        assertEq(slashed, false);

        vm.startPrank(user1);
        credit.approve(address(sgm), 100e18);
        sgm.stake(term1, 100e18);
        vm.stopPrank();

        (, , slashed ) = sgm.getRewards(user1, term1);
        assertEq(slashed, false);

        assertEq(profitManager.surplusBuffer(), 0);
        assertEq(profitManager.termSurplusBuffer(term1), 100e18);

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

        credit.mint(address(profitManager), 60e18);
        profitManager.notifyPnL(term1, 60e18);

        vm.startPrank(user1);
        sgm.unstake(term1, 50e18);
        vm.stopPrank();

        (, , slashed ) = sgm.getRewards(user1, term1);
        assertEq(slashed, false);

        assertEq(credit.balanceOf(user1), 50e18 + 20e18);//include rewards

        vm.warp(block.timestamp + 13);
        vm.roll(block.number + 1);

        profitManager.notifyPnL(term1, -20e18);
        guild.applyGaugeLoss(term1, address(sgm));

        (, , slashed ) = sgm.getRewards(user1, term1);
        assertEq(slashed, true);

        vm.warp(block.timestamp + 13);
        vm.roll(block.number + 1);

        vm.startPrank(user1);
        sgm.unstake(term1, 50e18);
        vm.stopPrank();

        (, , slashed ) = sgm.getRewards(user1, term1);
        assertEq(slashed, true);

        assertFalse(credit.balanceOf(user1) == 70e18 + 50e18, "balance = 70e + 50e18");
        assertEq(credit.balanceOf(user1), 70e18);

        vm.startPrank(user1);
        credit.approve(address(sgm), 10e18);
        sgm.stake(term1, 10e18);
        vm.stopPrank();

        (, , slashed ) = sgm.getRewards(user1, term1);
        assertEq(slashed, true);
    }

Tools Used

VScode

Recommended Mitigation Steps

The state of userStake should be updated before if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) part.

userStake = _stakes[user][term];

Assessed type

Other