Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Low Tawny Fox - Fee Validation Issue in Constructor and Setter #733

Open
sherlock-admin2 opened this issue Dec 5, 2024 · 0 comments
Open

Comments

@sherlock-admin2
Copy link
Contributor

Low Tawny Fox

Medium

Fee Validation Issue in Constructor and Setter

Summary

A vulnerability exists in the EthosVouch::initialize function where there is no check to ensure that the sum of fees is set correctly. This could result in the total fees exceeding the intended limit of 10%. Additionally, if all fees are set to a value greater or equal then 3.4 ETH in the EthosVouch::initialize, attempts to lower them via the setter function will revert.

Root Cause

The EthosVouch::initialize function does not validate that the total sum of fees is within the acceptable threshold (less than 10%). As a result, incorrect fee values can be set initially. Furthermore, the setter function does not handle cases where the fees are already set too high, causing it to revert when trying to decrease them.

Internal pre-conditions

The contract allows setting fees through the EthosVouch::initialize function without validation for the total fee sum.

Attack Path

During initialization of the contract, EthosVouch::initialize function is used to set fees without validating the total sum, allowing for fees to be set above the allowed threshold.
If all fees are set to an invalid value (e.g., 3.4 ETH), the setter function will fail when trying to lower them, locking the contract in an inconsistent state.

Impact

The contract becomes unusable if fees are set incorrectly during deployment. The failure of the setter function to adjust fees when they exceed the allowed limit results in a locked state where fee adjustments are no longer possible, rendering the contract inflexible and inefficient.

Mitigation

Add fee check in initialize function

function initialize(
    address _owner,
    address _admin,
    address _expectedSigner,
    address _signatureVerifier,
    address _contractAddressManagerAddr,
    address _feeProtocolAddress,
    uint256 _entryProtocolFeeBasisPoints,
    uint256 _entryDonationFeeBasisPoints,
    uint256 _entryVouchersPoolFeeBasisPoints,
    uint256 _exitFeeBasisPoints
  ) external initializer {
    __accessControl_init(
      _owner,
      _admin,
      _expectedSigner,
      _signatureVerifier,
      _contractAddressManagerAddr
    );

    __UUPSUpgradeable_init();
    if (_feeProtocolAddress == address(0)) revert InvalidFeeProtocolAddress();
    protocolFeeAddress = _feeProtocolAddress;
    entryProtocolFeeBasisPoints = _entryProtocolFeeBasisPoints;
    entryDonationFeeBasisPoints = _entryDonationFeeBasisPoints;
    entryVouchersPoolFeeBasisPoints = _entryVouchersPoolFeeBasisPoints;
    exitFeeBasisPoints = _exitFeeBasisPoints;
+ checkFeeExceedsMaximum(0, 0);
    configuredMinimumVouchAmount = ABSOLUTE_MINIMUM_VOUCH_AMOUNT;
    maximumVouches = 256;
    unhealthyResponsePeriod = 24 hours;
  }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant