Fit Lavender Cobra - Admins can set total fees to exceed 10% of BASIS_POINT_SCALE

[sherlock-admin2]

Fit Lavender Cobra

Medium

Admins can set total fees to exceed 10% of BASIS_POINT_SCALE

Summary

An incorrect value for MAX_TOTAL_FEES, allows cumulative fees to exceed the intended 10% of the BASIS_POINT_SCALE. This issue arises because MAX_TOTAL_FEES is set to 10000, representing 100% instead of 10%. Admins can unintentionally (or maliciously) configure fee percentages that result in excessive fees being charged to users.

From doc:

Maximum total fees cannot exceed 10%

Root Cause

In EthosVouch.sol, the MAX_TOTAL_FEES constant is set to 10000 (see here) instead of 1000, which represents 10% of the BASIS_POINT_SCALE. This discrepancy allows cumulative fees (entry protocol fee, exit fee, donation fee, and vouchers pool fee) to reach 100% rather than the intended 10%.

Internal pre-conditions

1. Admins set the following fee values (as an example, admin can set one fee to 100%):

• Entry protocol fee: 5000 basis points (50%).
• Donation fee: 2500 basis points (25%).
• Vouchers pool fee: 2500 basis points (25%).

2. The contract incorrectly calculates total fees using the oversized MAX_TOTAL_FEES.

External pre-conditions

None. This issue arises solely from internal misconfiguration.

Attack Path

1. Admins call setEntryProtocolFeeBasisPoints(5000).
2. Admins call setEntryDonationFeeBasisPoints(2500).
3. Admins call setEntryVouchersPoolFeeBasisPoints(2500).
4. A user interacts with the contract, incurring a total fee of 100% instead of 10%.

Impact

Users are charged excessive fees due to the miscalculated fee limit, resulting in financial loss. The discrepancy could also undermine trust in the protocol and harm its reputation.

PoC

Here’s a test case that demonstrates the issue (use it as part of ethos/packages/contracts/test/vouch/vouch.fees.test.ts file):

  it('Maximum total fees exceed 10%', async () => {
    const entryFee = 5000n;
    const donationFee = 2500n;
    const vouchIncentives = 2500n;
  
    await deployer.ethosVouch.contract
      .connect(deployer.ADMIN)
      .setEntryProtocolFeeBasisPoints(entryFee);
  
    await deployer.ethosVouch.contract
      .connect(deployer.ADMIN)
      .setEntryDonationFeeBasisPoints(donationFee);
  
    await deployer.ethosVouch.contract
      .connect(deployer.ADMIN)
      .setEntryVouchersPoolFeeBasisPoints(vouchIncentives);
  
    const amount = 100000000000000000n; // 0.1 ETH
    const { vouchId } = await userA.vouch(userB, { paymentAmount: amount });
    const balance = await userA.getVouchBalance(vouchId);
  
    const feeTaken = amount - balance;
    const feeAt10Percent = (amount * 1000n) / 10000n;
  
    expect(feeTaken).to.be.gt(feeAt10Percent);
  });

Mitigation

Change the value of MAX_TOTAL_FEES to 1000 to reflect 10% of the BASIS_POINT_SCALE.

uint256 public constant MAX_TOTAL_FEES = 1000; // Represents 10%
uint256 public constant BASIS_POINT_SCALE = 10000; // Basis point scale