018.md

Colossal Chiffon Urchin

Medium

updateDonationRecipient can be ddosed

Summary

updateDonationRecipient requires that newRecipient address doesn't have any rewards, but some users can add reward to it.

Root Cause

updateDonationRecipient requires empty donationRecipient

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

    // if the new donation recipient has a balance, do not allow overwriting
    // this is so rare, do we really need a custom error?
-->    require(donationEscrow[newRecipient] == 0, "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);
  }

but donationRecipient can be set arbitrary by anyone who can create market

  function _createMarket(
    uint256 profileId,
    address recipient,
    uint256 marketConfigIndex
  ) private nonReentrant {
...
    donationRecipient[profileId] = recipient;
...

contracts/ReputationMarket.sol#L341

and ddos it by buying 1 vote and sending fees to that address reward

  function applyFees(
    uint256 protocolFee,
    uint256 donation,
    uint256 marketOwnerProfileId
  ) private returns (uint256 fees) {
->    donationEscrow[donationRecipient[marketOwnerProfileId]] += donation;
    if (protocolFee > 0) {
      (bool success, ) = protocolFeeAddress.call{ value: protocolFee }("");
      if (!success) revert FeeTransferFailed("Protocol fee deposit failed");
    }
    fees = protocolFee + donation;
  }

ReputationMarket.sol#L1121

user can withdraw it with withdrawDonations to make 0 but attacker can do the same again

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

attacker sees that updateDonationRecipient called and buys 1 vote -> ddoes updateDonationRecipient

Impact

updateDonationRecipient will be ddosed which lead to users will not be able to receive funds in case of emergency

PoC

No response

Mitigation

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

    // if the new donation recipient has a balance, do not allow overwriting
    // this is so rare, do we really need a custom error?
-    require(donationEscrow[newRecipient] == 0, "Donation recipient has balance");
+    require(newRecipient != newRecipient, "Donation recipient has balance"); // @audit avoid doubling on the same address below attack

    // 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);
  }