075.md

Broad Grey Dragon

High

Race Condition Risk in applyFees Function of ReputationMarket.sol

Summary

https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L1086-L1097 The applyFees function in ReputationMarket.sol is vulnerable to a race condition when modifying the donationEscrow mapping. Specifically, the line:

donationEscrow[donationRecipient[marketOwnerProfileId]] += donation;

This operation can lead to inconsistent or incorrect balances if multiple transactions attempt to modify the same recipient's escrow balance concurrently. This is due to the possibility of parallel updates to the same entry in the mapping, which could result in race conditions.

Vulnerability Details

Severity: High Likelihood: Medium to High Impact: High

Issue Description:

In the applyFees function of ReputationMarket.sol, the following operation updates the donation escrow balance:

donationEscrow[donationRecipient[marketOwnerProfileId]] += donation;

This line modifies the donationEscrow mapping by adding the donation amount to the balance of a recipient identified by marketOwnerProfileId.

However, there is a race condition risk here when multiple transactions attempt to interact with the same marketOwnerProfileId and modify the donationEscrow mapping concurrently. A race condition arises because:

The balance of the recipient is updated using the += operator, which reads the current value and updates it by adding the donation amount. If two transactions happen at nearly the same time (concurrent transactions involving the same marketOwnerProfileId), both may read the same initial value of the recipient's balance and then add the donation amount. This results in only one of the updates being successfully applied, and the other being overwritten. As a result, the recipient may end up with less than the expected donation, or donations may be skipped altogether.

Example of the Race Condition:

Transaction A and Transaction B both interact with the contract for the same marketOwnerProfileId. Both transactions read the initial value of the recipient’s balance in donationEscrow[donationRecipient[marketOwnerProfileId]]. Both add the same donation amount, but since they both use the same read value, only one transaction’s update is stored, resulting in a loss of part of the expected donation.

Potential Impact:

Loss of funds: If two or more transactions modify the same recipient's donation escrow, the donations from each transaction may be undercounted. Inconsistent state: The contract could end up with an incorrect or inconsistent donation balance for the recipient. Economic Exploit: Malicious actors could exploit this by triggering simultaneous transactions to intentionally underreport or misappropriate donations.

PoC

PoC Steps:

Scenario Setup:

Assume that two or more transactions are initiated nearly simultaneously, each calling the applyFees function for the same marketOwnerProfileId (i.e., the same market). The donationEscrow mapping holds the donations for each market owner, and each call to applyFees will update this value. Vulnerable Interaction:

Transaction 1 and Transaction 2 attempt to apply fees for the same market at nearly the same time. Transaction 1 first checks the donationRecipient mapping, and Transaction 2 does the same. Both transactions calculate the donation, adding it to the donationEscrow for the given marketOwnerProfileId. Concurrent Updates:

Since these transactions are not synchronized and both access the donationEscrow mapping at nearly the same time, Transaction 1 and Transaction 2 might both end up updating the donationEscrow mapping without accounting for each other's changes. This could lead to double counting or missing donations, depending on the exact sequence of execution.

Outcome of the Race Condition:

If the race condition is triggered, the final donationEscrow balance for the marketOwnerProfileId could be incorrect (e.g., too high or too low), affecting the protocol fee collection and escrow withdrawal process.

PoC Code Example:

// Assume two transactions are initiated with the same marketOwnerProfileId

// Transaction 1
applyFees(protocolFeeAmount1, donationAmount1, marketOwnerProfileId);

// Transaction 2
applyFees(protocolFeeAmount2, donationAmount2, marketOwnerProfileId);

// Here, the `donationEscrow[marketOwnerProfileId]` could be incorrectly updated
// depending on the race condition, leading to wrong donation amounts.

In this example:

Transaction 1 and Transaction 2 both try to update the donationEscrow[marketOwnerProfileId] mapping at the same time. Since Solidity does not handle the synchronization of these updates, Transaction 2 might overwrite the state set by Transaction 1, leading to the wrong final value. Exploitation Potential: Attacker's Goal: Manipulate the donation to favor themselves or steal funds by exploiting the race condition. If the attacker can trigger many concurrent transactions interacting with the same marketOwnerProfileId, they could cause the contract to fail to properly track the donation amounts, resulting in financial manipulation.

Recommendation:

Locking Mechanism: Use a mutex or reentrancy guard to prevent multiple concurrent interactions with the same marketOwnerProfileId by locking the donation update process. This will prevent race conditions from occurring and ensure that each transaction's state is handled sequentially.

Example of a simple locking mechanism:

// Define a mapping to track the "locked" state for each marketOwnerProfileId
mapping(uint256 => bool) public marketLock;

modifier lockMarket(uint256 marketOwnerProfileId) {
    require(!marketLock[marketOwnerProfileId], "Market is locked");
    marketLock[marketOwnerProfileId] = true;
    _;
    marketLock[marketOwnerProfileId] = false;
}

function applyFees(
    uint256 protocolFee,
    uint256 donation,
    uint256 marketOwnerProfileId
) private lockMarket(marketOwnerProfileId) returns (uint256 fees) {
    // Existing logic for handling fees
}

This ensures that while one transaction is processing for a specific marketOwnerProfileId, no other transaction can concurrently modify the donation escrow, thus preventing the race condition.