Orbiting Ruby Ladybug - there is indeed a logical error in the fee calculation within the _calculateBuy and buyVotes functions

[sherlock-admin2]

Orbiting Ruby Ladybug

High

there is indeed a logical error in the fee calculation within the _calculateBuy and buyVotes functions

Summary

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L942
Double Deduction of Fees:
First Deduction: Fees are subtracted when calculating fundsAvailable in _calculateBuy and fundsReceived in _calculateSell.
Second Deduction: Fees are applied again in the buyVotes and sellVotes functions through the applyFees function.
Incorrect Market Funds Accounting:
The marketFunds variable is incorrectly updated with amounts that include fees already sent elsewhere, causing inconsistencies in the contract’s financial state.

Root Cause

Fee Calculation in _calculateBuy Function:

Initial Fee Deduction:

(fundsAvailable, protocolFee, donation) = previewFees(funds, true);

previewFees subtracts protocolFee and donation from funds to calculate fundsAvailable.
This means the fees are deducted upfront, reducing the amount available for purchasing votes.

Vote Purchase Loop:

while (fundsAvailable >= votePrice) {
    fundsAvailable -= votePrice;
    fundsPaid += votePrice;
    votesBought++;

    market.votes[isPositive ? TRUST : DISTRUST] += 1;
    votePrice = _calcVotePrice(market, isPositive);
}

fundsPaid accumulates the cost of votes purchased (excluding fees).

Adding Fees Back to fundsPaid:

fundsPaid += protocolFee + donation;

Fees are added back to fundsPaid, which represents the total cost including fees.

Double Counting Fees:
Fees are deducted upfront when calculating fundsAvailable.
Then, fees are added back to fundsPaid.
In the buyVotes function, applyFees is called, which transfers the fees, effectively deducting them again.
Fee Application in buyVotes Function:

Applying Fees Again:

applyFees(protocolFee, donation, profileId);

The applyFees function transfers the protocolFee and updates the donationEscrow, effectively deducting these fees from the user’s total funds a second time.

Refund Calculation:

uint256 refund = msg.value - fundsPaid;

The fundsPaid already includes the fees, and since applyFees has already deducted fees, the user ends up overcharged.

Updating Market Funds:

marketFunds[profileId] += fundsPaid;

The marketFunds includes the fees, but since the fees have been transferred elsewhere, this leads to an inflated marketFunds balance that doesn’t reflect the actual funds in the contract.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Users Overcharged:
Buyers pay the fees twice, reducing the number of votes they can purchase and overpaying in total.
Sellers receive less ETH than they should when selling votes.
Market Funds Misrepresented:
The marketFunds variable incorrectly includes fees that have been transferred out, leading to an inaccurate representation of the contract’s balance.
This could cause issues in functions that rely on marketFunds, such as withdrawGraduatedMarketFunds.

PoC

No response

Mitigation

No response