Skip to content

Latest commit

 

History

History
73 lines (53 loc) · 2.68 KB

File metadata and controls

73 lines (53 loc) · 2.68 KB

Cheesy Aegean Squirrel

Medium

Increase Vouch includes Current Voucher in Reward Calculation

Summary

Incorrect reward distribution in _rewardPreviousVouchers will allow users to receive back part of their own fees when increasing vouche staked amount as the function includes the voucher's own balance in the reward calculation.

Root Cause

In EthosVouch.sol the _rewardPreviousVouchers function includes the current voucher's balance and vouch when calculating reward proportions, allowing them to receive back part of their own fee when decide to increase his vouch:

function _rewardPreviousVouchers(uint256 amount, uint256 subjectProfileId) internal returns (uint256) {
    // Calculate total including current voucher
    for (uint256 i = 0; i < totalVouches; i++) {
        if (!vouch.archived) {
            totalBalance += vouch.balance;
        }
    }
    
    // Distribute rewards including to current voucher
    for (uint256 i = 0; i < totalVouches && remainingRewards > 0; i++) {
        if (!vouch.archived) {
            uint256 reward = amount.mulDiv(vouch.balance, totalBalance, Math.Rounding.Floor);
            vouch.balance += reward;
        }
    }
}

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

User can save up money by making a two step vouch instead of one Say that the user originally want to vouch 1.0001 ETH, he can save money by following this steps:

  1. User creates minimum vouch (0.0001 ETH) for target
  2. Same user increases vouch significantly (e.g. 1 ETH)
  3. The vouchers pool fee from the increase is distributed proportionally including their own vouch
  4. User receives back part of their own fee based on their proportion of total vouches

Impact

The protocol suffers reduced fee collection as users can reclaim part of their own vouchers pool fees. This defeats the purpose of the fee which is meant to reward previous vouchers, not the current voucher.

PoC

No response

Mitigation

Exclude the current voucher's balance when calculating reward distribution in _rewardPreviousVouchers:

function _rewardPreviousVouchers(uint256 amount, uint256 subjectProfileId, uint256 currentVouchId) internal returns (uint256) {
    for (uint256 i = 0; i < totalVouches; i++) {
        Vouch storage vouch = vouches[vouchIds[i]];
        // Only include other vouches in total
        if (!vouch.archived && vouch.vouchId != currentVouchId) {
            totalBalance += vouch.balance;
        }
    }
    // Rest of function...
}