Skip to content

Latest commit

 

History

History
103 lines (67 loc) · 4.21 KB

File metadata and controls

103 lines (67 loc) · 4.21 KB

Beautiful Powder Falcon

Medium

Overestimation of Exit Fees in ReputationMarket::previewExitFees

Summary

Issue:

The previewExitFees function overestimates the exit fees because it incorrectly assumes that the proceedsBeforeFees value includes only the base price, while it actually includes the tax amount. This leads to an inflated protocolFee calculation, resulting in an incorrect totalProceedsAfterFees.

Explanation of Tax Calculation:

When buying and selling votes, the Ethos protocol applies tax differently, depending on whether the tax is added to the price (in the case of buyVotes) or removed from the price (in the case of sellVotes). Here's how the tax should be applied in each case:

  • Buying Votes (buyVotes): Tax is added to the purchase price. The user pays the price plus the tax.

$$ \text{Tax Amount} = \text{Purchase Price} \times \frac{\text{Tax Rate}}{100} $$ $$ \text{Total Price} = \text{Purchase Price} + \text{Tax Amount} $$

Example: If the purchase price is $100 and the tax rate is 5%, the user ends up paying $105, with $5 being tax.

  • Selling Votes (sellVotes): The price already includes tax, so the tax is subtracted from the total price to determine the amount the user should receive.

$$ \text{Tax Amount} = \text{Price Including Tax} \times \frac{\text{Tax Rate}}{100 + \text{Tax Rate}} $$ $$ \text{Price Before Tax} = \text{Price Including Tax} - \text{Tax Amount} $$

Example: If the price including tax is $105 and the tax rate is 5%, the tax component is $5, leaving $100 as the amount to be paid to the user.

The Problem with previewExitFees:

The issue with the current implementation of previewExitFees is that it calculates the protocol fee assuming the proceedsBeforeFees does not include tax. However, proceedsBeforeFees already includes tax, leading to the calculation of the protocol fee based on an inflated value.

  function previewExitFees(
    uint256 proceedsBeforeFees
  ) private view returns (uint256 totalProceedsAfterFees, uint256 protocolFee) {
>>>    protocolFee = (proceedsBeforeFees * exitProtocolFeeBasisPoints) / BASIS_POINTS_BASE;

    totalProceedsAfterFees = proceedsBeforeFees - protocolFee;
  }

To correct this, the protocol fee calculation must exclude the tax already included in proceedsBeforeFees. The proper formula to account for this is:

  function previewExitFees(
    uint256 proceedsBeforeFees
  ) private view returns (uint256 totalProceedsAfterFees, uint256 protocolFee) {
-   protocolFee = (proceedsBeforeFees * exitProtocolFeeBasisPoints) / BASIS_POINTS_BASE;
+   protocolFee = (proceedsBeforeFees * exitProtocolFeeBasisPoints) / (100 * BASIS_POINTS_BASE + exitProtocolFeeBasisPoints);

    totalProceedsAfterFees = proceedsBeforeFees - protocolFee;
  }

Root Cause

In ReputationMarket::previewExitFees, the protocolFee is calculated as if the tax is not already included in the proceedsBeforeFees variable.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

The current implementation of the previewExitFees function overestimates the protocolFee and underestimates the totalProceedsAfterFees, leading to a loss of funds for the user. For instance, if proceedsBeforeFees is $105, the protocolFee is calculated as $5.25 (instead of $5), which is 5% higher than the correct fee amount. This overestimation results in the user receiving less than they should.

PoC

No response

Mitigation

To correct this, the protocol fee calculation must exclude the tax already included in proceedsBeforeFees. The proper formula to account for this is:

  function previewExitFees(
    uint256 proceedsBeforeFees
  ) private view returns (uint256 totalProceedsAfterFees, uint256 protocolFee) {
-   protocolFee = (proceedsBeforeFees * exitProtocolFeeBasisPoints) / BASIS_POINTS_BASE;
+   protocolFee = (proceedsBeforeFees * exitProtocolFeeBasisPoints) / (100 * BASIS_POINTS_BASE + exitProtocolFeeBasisPoints);

    totalProceedsAfterFees = proceedsBeforeFees - protocolFee;
  }