Gorples EVM Contracts Revision - Entangle Labs

In the xGorplesToken contract, the function finalizeRedeemFor is responsible for finalizing the redeeming process for a user by transferring the appropriate amount of GorplesCoin tokens and burning the excess. However, unlike the finalizeRedeem function, finalizeRedeemFor does not correctly decrement the user's xGorples balance after finalizing the redemption.

function finalizeRedeem(uint256 redeemIndex) public nonReentrant validateRedeem(msg.sender, redeemIndex) {
    RedeemInfo storage _userRedeem = userRedeems[msg.sender][redeemIndex];

    xBorpaBalances[msg.sender] = xBorpaBalances[msg.sender] - _userRedeem.xBorpaAmount;
    // .. // 
}

function finalizeRedeemFor(address _for) external onlyRole(SYSTEM) {
    uint256 len = userRedeems[_for].length;
    //E @audit missing decrement
    while (len > 0) {
        RedeemInfo storage _userRedeem = userRedeems[_for][len - 1];
        bool finalized = _easyFinalizeRedeem(
            _for,
            _userRedeem.xBorpaAmount,
            _userRedeem.endTime
        );

        if (finalized) {
            _deleteRedeemEntry(len - 1, _for);
        }

        len -= 1;
    }
}

This missing decrement operation results in incorrect xGorples balances, which can lead to the user having an inflated balance and potentially redeeming more Gorples tokens than they should be entitled to.

    function testFinalizeRedeemPOC(uint256 amount) public {
        //vm.assume(amount > 1e10 && amount <= INITIAL_LOOT_SUPPLY);
        amount = 10e9;
        // how can we transfer if transfers are banned ??? 
        // test invalid
        // borpa.transfer(user, amount);
        deal(address(borpa), user, 100e18, true); 

        console.log(borpa.balanceOf(user));

        vm.startPrank(user);
        borpa.approve(address(xBorpa), type(uint256).max);

        xBorpa.convert(amount);

        console.log("calling redeem");
        amount = amount * BURN_REMAINS / BURN_DENOMINATOR;

        xBorpa.redeem(amount);

        vm.warp(block.timestamp + xBorpa.REDEEM_DURATION());

        uint before = borpa.balanceOf(user);

        uint256 len = xBorpa.getUserRedeemsLength(user);
        console.log("len", len);

        (uint256 xbrp, uint256 time) = xBorpa.getUserRedeem(user, 0);
        console.log("xbrp", xbrp, "time", time);

        assertEq(xbrp, amount);
        assertEq(len, 1);

        console.log("Calling finzalize redeem");
        xBorpa.finalizeRedeem(0);
        amount = amount * BURN_REMAINS / BURN_DENOMINATOR;

        uint afterBalance = borpa.balanceOf(user) - before;

        assertApproxEqRel(afterBalance, amount, 1e10);
        assertEq(xBorpa.balanceOf(address(xBorpa)), 0);
        assertEq(xBorpa.getUserRedeemsLength(user), 0);
        assertEq(xBorpa.xBorpaBalances(user), 0);
    }

    function testFinalizeRedeemPOCFor(uint256 amount) public {
        //vm.assume(amount > 1e10 && amount <= INITIAL_LOOT_SUPPLY);
        amount = 10e9;
        // how can we transfer if transfers are banned ??? 
        // test invalid
        // borpa.transfer(user, amount);
        deal(address(borpa), user, 100e18, true); 

        console.log(borpa.balanceOf(user));

        vm.startPrank(user);
        borpa.approve(address(xBorpa), type(uint256).max);

        xBorpa.convert(amount);

        console.log("calling redeem");
        amount = amount * BURN_REMAINS / BURN_DENOMINATOR;

        xBorpa.redeem(amount);

        vm.warp(block.timestamp + xBorpa.REDEEM_DURATION());

        uint before = borpa.balanceOf(user);

        uint256 len = xBorpa.getUserRedeemsLength(user);
        console.log("len", len);

        (uint256 xbrp, uint256 time) = xBorpa.getUserRedeem(user, 0);
        console.log("xbrp", xbrp, "time", time);

        assertEq(xbrp, amount);
        assertEq(len, 1);

        console.log("Calling finzalize redeem For");
        vm.stopPrank();
        vm.startPrank(admin);
        xBorpa.finalizeRedeemFor(address(user));
        vm.stopPrank();

        amount = amount * BURN_REMAINS / BURN_DENOMINATOR;

        uint afterBalance = borpa.balanceOf(user) - before;

        assertApproxEqRel(afterBalance, amount, 1e10);
        assertEq(xBorpa.balanceOf(address(xBorpa)), 0);
        assertEq(xBorpa.getUserRedeemsLength(user), 0);
        assertEq(xBorpa.xBorpaBalances(user), 0);
    }

As it can be seen in the tests, the one using finalizeRedeem work because xGorplesToken decrement is implemented in it but the test using finalizeRedeemFor does not work and the assertion cannot be verified:

It is recommended to add the missing decrement operation to the finalizeRedeemFor function to ensure the user's xGorples balance is correctly updated.

Remediation Plan

SOLVED: The missing decrement line has been added to finalizeRedeemFor.