095.md

Cool Mango Bobcat

Medium

Accounting Discrepancy in Fee Handling Leads to Failed Withdrawals and Fund Lockup

Summary

The ReputationMarket contract contains a critical accounting vulnerability in its fee handling mechanism. The core issue stems from an architectural disconnect between funds tracking and fee processing mechanisms.

The contract maintains a marketFunds mapping to track available funds per market, but processes fees (both protocol fees and donations) independently of these fund updates in the applyFees function. This results in fees being transferred or escrowed without corresponding deductions from marketFunds.

This architectural flaw manifests in system unreliability and potential fund inaccessibility. The marketFunds accounting becomes inaccurate as fees are processed without corresponding balance updates. While Ethereum's native balance checks prevent actual over-withdrawal of funds, the incorrect accounting in marketFunds leads to failed withdrawal attempts during market graduation. The discrepancy makes it impossible to maintain proper accounting of available funds versus escrowed fees.

While funds cannot be directly stolen due to Ethereum's balance checks, the accounting discrepancy leads to system unreliability and potential fund lockup.

Proof of Concept

The vulnerability can be demonstrated through a typical trading scenario that exposes the accounting discrepancy:

// Initial state
marketFunds[profileId] = 0 ETH
contract.balance = 0 ETH

// User buys votes with 1 ETH
// purchaseCostBeforeFees = 0.95 ETH
// protocolFee = 0.03 ETH
// donation = 0.02 ETH

// In buyVotes:
marketFunds[profileId] += purchaseCostBeforeFees; // now 0.95 ETH

// In applyFees:
donationEscrow[recipient] += donation; // 0.02 ETH escrowed
protocolFeeAddress.call{value: protocolFee}(""); // 0.03 ETH transferred

// Final state
marketFunds[profileId] = 0.95 ETH       // Incorrect
contract.balance = 0.97 ETH             // Actual (1 - 0.03 protocol fee)
donationEscrow[recipient] = 0.02 ETH

When market graduation occurs, this discrepancy leads to failed withdrawals:

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

function withdrawGraduatedMarketFunds(uint256 profileId) {
    // Attempts to withdraw 0.95 ETH
    _sendEth(marketFunds[profileId]); // Will fail as only 0.97 ETH available
                                     // and 0.02 ETH needed for escrow
}

Recommended mitigation steps

The solution requires a fundamental restructuring of the fee handling mechanism to maintain strict accounting accuracy. The applyFees function should be modified to explicitly deduct fees from market funds before processing them:

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

This should be complemented by implementing a fund invariant checking system that validates the consistency between contract balance and recorded funds:

function checkFundInvariants() internal view {
    uint256 totalMarketFunds = 0;
    uint256 totalEscrow = 0;
    
    for (uint256 i = 0; i < markets.length; i++) {
        totalMarketFunds += marketFunds[i];
    }
    
    for (address recipient in donationRecipients) {
        totalEscrow += donationEscrow[recipient];
    }
    
    require(
        address(this).balance >= totalMarketFunds + totalEscrow,
        "Fund invariant violated"
    );
}

These system-wide invariant checks should be applied consistently across all fund-handling operations through a modifier:

modifier withFundCheck() {
    _;
    checkFundInvariants();
}

This comprehensive approach ensures robust fund tracking while maintaining the contract's core functionality. The solution maintains a strict correlation between actual contract balances and recorded fund states, preventing accounting discrepancies that could lead to failed operations or locked funds.