Appeals in Challenges Can Be Manipulated Due to Improper Handling of `firstFunded` Variable

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd13ab2125a9338b7e96796da5b13a9c76b6f5ac10f527d34b23cc7228ba51121 Severity: high

Description: Description:
When a user challenges a claim using the challengeRequest function, a new round is created for the request.challenges[challengeId] variable:

    challenge.lastRoundId++;

Since this is a new round, it has no pre-existing data and is primarily used to store appeal deposits.

When a user calls the fundAppeal function with a specific side and provides sufficient msg.value to fund an appeal, a local variable firstFunded is created:

    Party firstFunded = round.sideFunded;
    require(_side != firstFunded);

In this context, the firstFunded variable is initially set to Party.None because the round being referenced in the fundAppeal function is a new round. By default, a new round's sideFunded variable has the value Party.None.

This variable is subsequently checked against Party.None after the fundAppeal function invokes the _contribute function to handle contributions and update the round:

    if (
        _contribute(
            disputeData.humanityId,
            disputeData.requestId,
            disputeData.challengeId,
            challenge.lastRoundId,
            _side,
            totalCost
        ) &&
        // If firstFunded was assigned, it means one side was funded, and if this side also gets fully funded, an appeal can be created.
        firstFunded != Party.None
    ) {

However, firstFunded is a local variable created within the fundAppeal function and its value is set before the _contribute function is called. As a result, it does not get updated during the execution of _contribute.

Therefore, when the firstFunded variable is compared to Party.None, the condition firstFunded != Party.None will always evaluate to false. Consequently, an appeal will not be created.

This issue prevents appeals from ever being initiated, which means that challenges cannot be contested further, effectively breaking a core functionality of the protocol — the appeals system.

The _contribute function is executed during the check of the above if condition. As explained earlier, the code within the if statement will never execute. This leads to a scenario where, if a user provides sufficient funds when calling the fundAppeal function, the _contribute function sets the current round's sideFunded variable according to the side chosen by the caller:

    if (requiredAmount <= msg.value) {
        contribution = requiredAmount;
        remainingETH = msg.value - requiredAmount;

        paidInFull = true;
        round.sideFunded = round.sideFunded == Party.None ? _side : Party.None;
    }

Due to the aforementioned reasons, the if condition inside the fundAppeal function will not execute, and thus the current round does not advance to a new one:

    if (_contribute(disputeData.humanityId, disputeData.requestId, disputeData.challengeId, challenge.lastRoundId, _side, totalCost) &&
        // If firstFunded was assigned, it means one side was funded, and if this side also gets fully funded, an appeal can be created.
        firstFunded != Party.None
    ) {
        IArbitrator(_arbitrator).appeal{value: appealCost}(
            _disputeId,
            arbitratorDataHistory[request.arbitratorDataId].arbitratorExtraData
        );
@>        challenge.lastRoundId++;

        // Subtract the costs from the total staked contributions
        round.feeRewards = round.feeRewards.subCap(appealCost);

        emit AppealCreated(IArbitrator(_arbitrator), _disputeId);
    }

As a result, the current round's sideFunded value is set based on the caller's choice in the fundAppeal function.

Later, when the rule function is called and executed, since the round is not updated as mentioned, the winner of the dispute will always be determined by the side chosen by the caller of fundAppeal:

    Party resultRuling = Party(_ruling);
    // The ruling is inverted if the loser paid its fees.
    if (round.sideFunded == Party.Requester)
        // If one side paid its fees, the ruling is in its favor. Note that if the other side had also paid, an appeal would have been created.
        resultRuling = Party.Requester;
    else if (round.sideFunded == Party.Challenger) resultRuling = Party.Challenger;

This flaw allows any malicious user to front-run the rule function by calling the fundAppeal function with their desired winner, effectively allowing them to determine the outcome of the dispute regardless of the arbitrator's actual ruling, thereby bypassing the challenge system.

Scenario:

1. Alice challenges a humanity claim using the challengeRequest function, which creates a new round by incrementing challenge.lastRoundId. This new round has no data, serving primarily to hold appeal deposits.
2. Bob, a participant in the challenge, calls the fundAppeal function with sufficient msg.value to make an appeal. Inside this function, a local variable firstFunded is initialized to round.sideFunded. Since this is a new round, round.sideFunded is set to Party.None by default. Thus, firstFunded is initialized to Party.None.
3. The fundAppeal function calls the _contribute function to update the round and contribute to the appeal. The firstFunded variable is checked after the _contribute call to see if it differs from Party.None. However, since firstFunded is a local variable and not updated by _contribute, it retains its initial value (Party.None). Thus, the condition firstFunded != Party.None always evaluates to false, preventing the appeal from being created.
4. After calling fundAppeal, the round.sideFunded is updated based on the caller’s chosen side (either Party.Requester or Party.Challenger).
5. When the rule function is called to finalize the dispute, the winner is determined based on the round.sideFunded value.

Solution:
Convert firstFunded to a State Variable.

[clesaege]

For an appeal to be created, both sides should be funded. If only one side is funded, this side will be considered the winning side irrespective of the ruling.

In your example everything you describes works as intended, if no one takes a side different than Bob, his side will win.

If Carl funds a side opposing to Bob, un the subsequent fundAppeal we will have firstFunded be Bob and the condition to create an appeal will be satisfied.