Skip to content

Latest commit

 

History

History
86 lines (62 loc) · 3.32 KB

055.md

File metadata and controls

86 lines (62 loc) · 3.32 KB

Slow Tan Swallow

Medium

calcFee calculates the fee wrongly

Summary

calcFee does some math in order to calculate the fee. However the math is reversed for fee addition (total = balance + fee), but the contract uses fee subtraction (balance = balance - fee, so that total = balance + fee).

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L975-L989

  function calcFee(uint256 total, uint256 feeBasisPoints) internal pure returns (uint256 fee) {
    // total - total * BASIS_POINT_SCALE / (BASIS_POINT_SCALE + feeBasisPoints)
    // total = 100 || feeBasisPoints = 10% (1000)
    // 100e18 - 100e18 * 10e3 / (10e3 + 1000) = 9.0909e18 -> ~10% lower fee
    return
      total -
      (total.mulDiv(BASIS_POINT_SCALE, (BASIS_POINT_SCALE + feeBasisPoints), Math.Rounding.Floor));
  }

This can be seen inside applyFees, where the toDeposit is just reduced by the fee, resulting in the user being charged the original total which he send as msg.value.

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L929-L965

  function applyFees(uint256 amount, bool isEntry, uint256 subjectProfileId) internal returns (uint256 toDeposit, uint256 totalFees) {
    if (isEntry) {
      // Calculate entry fees
      uint256 protocolFee = calcFee(amount, entryProtocolFeeBasisPoints);
      uint256 donationFee = calcFee(amount, entryDonationFeeBasisPoints);
      uint256 vouchersPoolFee = calcFee(amount, entryVouchersPoolFeeBasisPoints);

      // Distribute fees
      if (protocolFee > 0) {
        _depositProtocolFee(protocolFee);
      }
      if (donationFee > 0) {
        _depositRewards(donationFee, subjectProfileId);
      }
      if (vouchersPoolFee > 0) {
        vouchersPoolFee = _rewardPreviousVouchers(vouchersPoolFee, subjectProfileId);
      }
      totalFees = protocolFee + donationFee + vouchersPoolFee;
      toDeposit = amount - totalFees;

Root Cause

The impact of this is that the generated fees would be ~10% lower than expected. This would be true for the system (protocolFee and exitFee), while also for the vouchers (vouchersPoolFee) and the one they are vouching for (donationFee), as all of these use calcFee.

  function calcFee(uint256 total, uint256 feeBasisPoints) internal pure returns (uint256 fee) {
    // total - total * BASIS_POINT_SCALE / (BASIS_POINT_SCALE + feeBasisPoints)
    // total = 100 || feeBasisPoints = 10% (1000)
    // 100e18 - 100e18 * 10e3 / (10e3 + 1000) = 9.0909e18 -> ~10% lower fee
    return
      total -
      (total.mulDiv(BASIS_POINT_SCALE, (BASIS_POINT_SCALE + feeBasisPoints), Math.Rounding.Floor));
  }

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

The system looses funds due to the miscalculated fee.

PoC

No response

Mitigation

applyFees subtracts the fees from the total instead of charging more, which means that it works like a normal fee (taking part of user's deposits), however due to the reversed math the current implementation charges a lower amount than the one it should. Consider flipping back in order to increase profits and match user expectations (as vouchers and subjects earn less).