Skip to content

Latest commit

 

History

History
106 lines (69 loc) · 3.98 KB

File metadata and controls

106 lines (69 loc) · 3.98 KB

Attractive Carrot Pike

Medium

Wrong Revert Implemetation in ReputationMarket.sol contract. This tend to DOS attack.

Summary

In the updateDonationRecipient function of the ReputationMarket.sol contract, the wrong revert error ZeroAddress() is used instead of ZeroAddressNotAllowed() from ReputationMarketErrors.sol. This misimplementation results in unclear error messages and exposes the system to potential misuse, such as setting the protocol fee address to address(0).

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

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];
    donationEscrow[msg.sender] = 0;
    emit DonationRecipientUpdated(profileId, msg.sender, newRecipient);
  }

Root Cause

Wrong Revert Statement Implementation.

Internal Pre-conditions

address(0) is not explicitly prevented from being set as the updateDonationRecipient due to improper revert handling.

External Pre-conditions

1.The attacker execute the updateDonationRecipient function. 2. The protocol must be in a state where it is not paused (whenNotPaused)

Attack Path

1.The function fails to provide a clear error message due to the use of ZeroAddress(). 2. If not reverted, updateDonationRecipient is updated to address(0). 3. Future transactions relying on a valid updateDonationRecipient may fail, leading to a partial or complete denial of service.

Impact

  1. Vulnerability to DoS Attacks: Improper validation of address(0) can allow malicious or accidental misuse, potentially halting protocol operations.
  2. Poor User Experience: The unclear error message from ZeroAddress() reduces usability and debugging efficiency.

PoC

Manual Review

Mitigation

Replace the Revert ZeroAddress() with ZeroAddressNotAllowed() from ReputationMarketErrors.sol:625

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

       // 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];
       donationEscrow[msg.sender] = 0;
       emit DonationRecipientUpdated(profileId, msg.sender, newRecipient);
     }