021.md

Urban Obsidian Jay

High

The updateDonationRecipient() Function is Not Safe.

Summary

When a user with multiple markets calls the updateDonationRecipient() function, the user risks lossing funds.

Root Cause

https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L642

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

N/A

Impact

Potential loss of funds.

PoC

https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/EthosProfile.sol#L415 The vulnerability can be demonstrated through the following link, which shows it is possible that a deleted address from one profile can be re-registered to another profile. As a result, there are some users who have multiple markets.

    function updateDonationRecipient(
        uint256 profileId,
        address newRecipient
    ) public whenNotPaused nonReentrant {
        if (newRecipient == address(0)) revert ZeroAddress();

        // if the new donation recipient has a balance, do not allow overwriting
        if (donationEscrow[newRecipient] != 0)
        revert InvalidMarketConfigOption("Donation recipient has balance");

        // Ensure the sender is the current donation recipient
        if (msg.sender != donationRecipient[profileId]) revert InvalidProfileId();

        // Ensure the new recipient has the same Ethos profileId
        uint256 recipientProfileId = _ethosProfileContract().verifiedProfileIdForAddress(newRecipient);
        if (recipientProfileId != profileId) revert InvalidProfileId();

        // Update the donation recipient reference
        donationRecipient[profileId] = newRecipient;
        // Swap the current donation balance to the new recipient
        donationEscrow[newRecipient] += donationEscrow[msg.sender];
642     donationEscrow[msg.sender] = 0;
        emit DonationRecipientUpdated(profileId, msg.sender, newRecipient);
    }

At line 642, the function sets donationEscrow[msg.sender] to zero after transferring the funds to newRecipient. This can lead to unintended loss of funds, particularly for users with multiple markets.

Mitigation

Consider using the profileId as parameters for the donationEscrow.