Sparkly Ruby Rabbit - Reverting Fee/Donation Address

[sherlock-admin4]

Sparkly Ruby Rabbit

High

Reverting Fee/Donation Address

Summary

A malicious or careless admin will brick all buy/sell or donation withdrawals for market participants by setting fee or donation addresses to a contract that reverts on receiving ETH.

When fees (protocol fee) or donations are distributed, the contract calls something like:

(bool success, ) = protocolFeeAddress.call{ value: protocolFee }("");
if (!success) revert FeeTransferFailed("Protocol fee deposit failed");

Similarly, for donations:

(bool success, ) = msg.sender.call{ value: amount }("");
if (!success) revert FeeTransferFailed("Donation withdrawal failed");

If protocolFeeAddress or donation recipients are addresses (contracts) that revert on ETH reception—perhaps via a receive() or fallback() function—all transactions that attempt to transfer ETH there will revert, effectively halting buy/sell or donation withdrawal flows.

Impact
This breaks core functionality for users, as any buy/sell that must pay a fee (or a withdrawal that sends donations) fails and reverts. This is a Denial of Service scenario for trading or withdrawing donations.

Root Cause

In ReputationMarket.sol, applyFees(protocolFee, donation, profileId):

if (protocolFee > 0) {
  (bool success, ) = protocolFeeAddress.call{ value: protocolFee }("");
  if (!success) revert FeeTransferFailed("Protocol fee deposit failed");
}

Also, withdrawDonations():

(bool success, ) = msg.sender.call{ value: amount }("");
if (!success) revert FeeTransferFailed("Donation withdrawal failed");

No check is performed to confirm that the fee/donation recipient can safely receive ETH. An address with a fallback that reverts (intentionally or by design) causes all transfers to fail.

Internal Pre-conditions

1-Admin can call setProtocolFeeAddress(...) or set a donation recipient by creating a market or updating it.
2-The contract calls applyFees(...) or withdrawDonations() to transfer ETH to that address.

External Pre-conditions

1-The chosen feeAddress or donation recipient is a contract with a fallback or receive function that reverts on inbound ETH.
2-A user tries a trade or donation withdrawal, triggering the failing transfer.

Attack Path

1-Malicious Admin sets protocolFeeAddress to a contract that reverts when receiving ETH.
2-User calls buyVotes() or sellVotes():

• The code tries to send the fee to protocolFeeAddress.
• The target reverts, causing the entire transaction to revert.
  3-As a result, no one can complete a buy or sell in that market. Trading is entirely blocked.
  4-Alternatively, if a donation recipient with revert fallback is set, attempts to withdraw donations fail for that recipient.

Impact

Affected Party:
All traders needing to pay protocol fees.
Donation recipients if their own address reverts.

Result:
Denial of Service for any function that must transfer ETH to the reverter address.
Users cannot trade, the market effectively becomes non-functional.

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;

import "forge-std/Test.sol";
import "../src/ReputationMarket.sol";

contract RevertingReceiver {
    fallback() external payable {
        revert("I do not accept ETH");
    }
}

contract RevertingFeeAddressTest is Test {
    ReputationMarket rm;
    RevertingReceiver revertContract;

    function setUp() public {
        rm = new ReputationMarket();
        revertContract = new RevertingReceiver();
    }

    function testFeeAddressRevert() public {
        // Admin sets the protocol fee address to the reverting contract
        vm.prank(rm.owner()); // or admin
        rm.setProtocolFeeAddress(address(revertContract));

        // Now user tries to buy or sell
        vm.deal(address(this), 1 ether);
        vm.expectRevert(bytes("Protocol fee deposit failed"));
        rm.buyVotes{value: 1 ether}(12345, true, 10, 1);
        // Reverts because fee transfer fails
    }
}

When applyFees(...) tries to send ETH to revertContract, it triggers a revert.

Mitigation

1-Validate Fee Address
Require that protocolFeeAddress is an EOA or at least a contract that accepts ETH (test via try/catch).

2-Fallback Mechanism
If the transfer fails, store the fees in escrow or allow a retry by the admin rather than reverting the entire trade.

3-Governance & Safeguards
Restrict setProtocolFeeAddress to a safe list or require a multi-sig governance process.

4-Donation
Similarly, ensure the donation recipient can safely receive ETH or handle reverts gracefully.