Broad Grey Dragon
Medium
https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L621-L644 The updateDonationRecipient function allows the current donation recipient of a market to update the recipient address, transferring any existing donation balance to the new recipient. However, due to a lack of sufficient locking mechanisms, this process is vulnerable to a race condition.
The updateDonationRecipient function contains a critical flaw where two or more concurrent transactions can lead to an inconsistent state. The key steps in the process are:
The function checks the sender’s validity as the current donation recipient.
It updates the donationRecipient mapping to the new recipient.
The donation balance of the current recipient is transferred to the new recipient.
However, without atomicity, these operations are not synchronized, allowing an attacker to:
Call the function multiple times in quick succession using different newRecipient addresses.
Exploit a gap between the validation and state update to introduce conflicting or unintended recipient addresses.
The race condition arises because the state update for donationRecipient and donationEscrow is not protected by a mutex or other synchronization mechanism. Consequently, a malicious actor or a bot could execute overlapping transactions, disrupting the intended state.
Severity: Medium to High Likelihood: Medium Impact: High
The following steps outline a PoC to demonstrate the vulnerability:
Setup: Assume the msg.sender is the current donation recipient for profileId.
Execute Concurrent Transactions:
Transaction 1: updateDonationRecipient(profileId, newRecipient1)
Transaction 2: updateDonationRecipient(profileId, newRecipient2)
Outcome:
Both transactions pass the initial validation.
The donationRecipient mapping and donationEscrow balance are updated inconsistently, resulting in unexpected or incorrect states.
To mitigate the race condition, the following measures are recommended:
Use Reentrancy Guards: Implement a nonReentrant modifier to prevent overlapping calls to updateDonationRecipient.
Atomic State Updates: Ensure all updates to donationRecipient and donationEscrow occur atomically. This could involve:
Using local variables to temporarily store intermediate states.
Committing state changes only after all validations pass.
Transaction Locks: Introduce a per-profile lock mechanism to ensure that only one update process can occur for a given profileId at any time.
Event Monitoring: Emit detailed events for each stage of the update process to facilitate debugging and monitoring.
Code Example: Mitigation
function updateDonationRecipient(uint256 profileId, address newRecipient)
public
whenNotPaused
nonReentrant
{
if (newRecipient == address(0)) revert ZeroAddress();
if (donationEscrow[newRecipient] != 0) revert InvalidMarketConfigOption("Donation recipient has balance");
if (msg.sender != donationRecipient[profileId]) revert InvalidProfileId();
uint256 recipientProfileId = _ethosProfileContract().verifiedProfileIdForAddress(newRecipient);
if (recipientProfileId != profileId) revert InvalidProfileId();
// Atomic Update
donationEscrow[newRecipient] += donationEscrow[msg.sender];
donationEscrow[msg.sender] = 0;
donationRecipient[profileId] = newRecipient;
emit DonationRecipientUpdated(profileId, msg.sender, newRecipient);
}