038.md

Recumbent Shamrock Barracuda

Medium

Multiple fee miscalculation leads to inaccurate implementation of the fee model

Summary

The applyFees function miscalculates cumulative fees when multiple fees (protocol, donation, vouchers pool) are applied. This function does not guarantee that the fees will be a percentage of the actual deposit amount as it calculates each fee independently based on the original amount. This leads to incorrect fee distributions, resulting in fee overcharges and deposit undercharges and potential fund misallocations.

The calcFee function is to ensure that the sum of the deposit amount and the fee is the total fund, and the fee is a percentage of the deposit amount. In short, the fee is not a percentage of the total funds, but a percentage of the funds that are useful to the user, that is, the deposit amount. https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L975-L989

But, the following equation does not actually hold true: protocolFee = toDeposit * entryProtocolFeeBasisPoints / BASIS_POINT_SCALE donationFee = toDeposit * entryDonationFeeBasisPoints / BASIS_POINT_SCALE vouchersPoolFee = toDeposit * entryVouchersPoolFeeBasisPoints / BASIS_POINT_SCALE

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

Root Cause

The root cause is that the calcFee function does not calculate the fee from the actual value of toDeposit obtained by deducting the entire fee, but applies the individual fees independently to the original amount. This is because the fee calculation includes not only the funds actually used by the user, i.e. the deposit amount, but also other types of fees. In short, the protocol charges users fees even for the funds that are paid as fees for other type of fees.

PoC (Proof of Concept)

Example Scenario:

• Transaction amount: 1e18
• Fees: (according to vouch.fees.test.ts)
  • entryProtocolFeeBasisPoints = 50 (0.5%)
  • entryDonationFeeBasisPoints = 150 (1.5%)
  • entryVouchersPoolFeeBasisPoints = 200 (2%)

Current Implementation:

1. Protocol Fee: calcFee(1e18, 50) = 4,975,124,378,109,453
2. Donation Fee: calcFee(1e18, 150) = 14,778,325,123,152,710
3. Vouchers Pool Fee: calcFee(1e18, 200) = 19,607,843,137,254,902

Total fees: 4,975,124,378,109,453 + 14,778,325,123,152,710 + 19,607,843,137,254,902 = 39,361,292,638,517,065
Remaining for deposit: 1e18 - 39,361,292,638,517,065 = 960,638,707,361,482,935

Mismatch:

1. Protocol Fee: 960,638,707,361,482,935 * 50 / 1e18 = 4,803,193,536,807,414 => diff with 4,975,124,378,109,453
2. Donation Fee: 960,638,707,361,482,935 * 150 / 10000 = 14,409,580,610,422,244 => diff with 14,778,325,123,152,710
3. Vouchers Pool Fee: 960,638,707,361,482,935 * 200 / 10000 = 19,212,774,147,229,658 => diff with 19,607,843,137,254,902

User's total loss of toDeposit: (4,975,124,378,109,453 - 4,803,193,536,807,414) + (14,778,325,123,152,710 - 14,409,580,610,422,244) + (19,607,843,137,254,902 - 19,212,774,147,229,658) = 935,744,344,057,749 Percentage: 935,744,344,057,749 * 100 / 1e18 = 0.094%

This means that 0.094% of user funds were paid as fees instead of the deposit amount.

Impact

Failure to accurately implement the fee model has the following impacts:

• Excess fees are sent to the protocol, donation pool or voucher pool:
• The user's deposit amount will be reduced accordingly:

The severity is rated as medium based on the estimate of user loss.

Mitigation

Refactor Fee Calculation to Aggregate and Distribute Proportionally

Modify the applyFees function to calculate the total fee first, using the combined basis points of all applicable fees. Then distribute the total fee proportionally to each component (e.g., protocol, donation, vouchers pool). This ensures accurate allocation without requiring sequential calculations.

Implementation Steps:

1. Calculate Total Fee
   Use the sum of all basis points for the relevant fees to calculate the total fee in one step.

2. Distribute Total Fee Proportionally
   Allocate the total fee to each fee component based on its respective basis points.

Although it is not reflected in the code, first check if vouchersPoolFee is needed and reflect entryVouchersPoolFeeBasisPoints in totalBasisPoints only if necessary.

Updated Implementation:

function applyFees(
    uint256 amount,
    bool isEntry,
    uint256 subjectProfileId
) internal returns (uint256 toDeposit, uint256 totalFees) {
    if (isEntry) {
        // Aggregate all entry fees
        uint256 totalBasisPoints = entryProtocolFeeBasisPoints +
                                   entryDonationFeeBasisPoints +
                                   entryVouchersPoolFeeBasisPoints;
                                   
        // Calculate total fee
        totalFees = calcFee(amount, totalBasisPoints);

        // Proportional distribution of total fees
        uint256 protocolFee = (totalFees * entryProtocolFeeBasisPoints) / totalBasisPoints;
        uint256 donationFee = (totalFees * entryDonationFeeBasisPoints) / totalBasisPoints;
        uint256 vouchersPoolFee = (totalFees * entryVouchersPoolFeeBasisPoints) / totalBasisPoints;

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

        toDeposit = amount - totalFees;

    } else {
        // Exit fee calculation remains unchanged
        totalFees = calcFee(amount, exitFeeBasisPoints);
        if (totalFees > 0) {
            _depositProtocolFee(totalFees);
        }
        toDeposit = amount - totalFees;
    }

    return (toDeposit, totalFees);
}