Insufficient boundary check for `buyFee` in `BondingCurveBase_v1.sol::setBuyFee`.

[hats-bug-reporter[bot]]

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0x281e417d0c9e6133741d79c90249a98445f4a2b0f43cc3d58dfb4b3482c7e506 Severity: low

Description: Description\

In BondingCurveBase_v1.sol::setBuyFee:

function setBuyFee(uint _fee) external virtual onlyOrchestratorAdmin {
        _setBuyFee(_fee);
    }

In BondingCurveBase_v1.sol::_setBuyFee:

function _setBuyFee(uint _fee) internal virtual {
        _validateWorkflowFee(_fee);
        emit BuyFeeUpdated(_fee, buyFee);
        buyFee = _fee;
    }

In BondingCurveBase_v1.sol::_validateWorkflowFee:

function _validateWorkflowFee(uint _workflowFee) internal pure {
        if (_workflowFee > BPS) {
            revert Module__BondingCurveBase__InvalidFeePercentage();
        }
    }

When setting new buyFee, the contract checks that the value is less than or equal to BPS (=10_000) value.

Now, take a look at the function BondingCurveBase_v1.sol::_calculateNetAndSplitFees:

function _calculateNetAndSplitFees(
        uint _totalAmount,
        uint _protocolFee,
        uint _workflowFee
    )
        internal
        pure
        returns (uint netAmount, uint protocolFeeAmount, uint workflowFeeAmount)
    {
        if ((_protocolFee + _workflowFee) > BPS) {
            revert Module__BondingCurveBase__FeeAmountToHigh();
        }

The contract checking _protocolFee + _workflowFee is less than or equal BPS.

_calculateNetAndSplitFees function is used in the following functions:

• BondingCurveBase_v1.sol::calculatePurchaseReturn

• BondingCurveBase_v1.sol::_buyOrder

Attack Scenario\

If the buyFee is set to BPS, the function will always reverts if _protocolFee > 0 in _calculateNetAndSplitFees function, causing denial of service situation.

Attachments

NA

1. Proof of Concept (PoC) File

Manual Analysis

2. Revised Code File (Optional)

Consider making the following changes BondingCurveBase_v1.sol::_validateWorkflowFee :

function _validateWorkflowFee(uint _workflowFee) internal pure {
++      (
++          /* collateralreasury */
++          ,
++          /* issuanceTreasury */
++          ,
++          uint collateralBuyFeePercentage ,
++          /* uint issuanceBuyFeePercentage */
++      ) = _getFunctionFeesAndTreasuryAddresses(
++          bytes4(keccak256(bytes("_buyOrder(address, uint, uint)")))
++      );
++      if (_workflowFee + collateralBuyFeePercentage > BPS) {
--        if (_workflowFee  > BPS) {
            revert Module__BondingCurveBase__InvalidFeePercentage();
        }
    }

[PlamenTSV]

It would be an owner mistake to set both fees with incompatible values. I think OOS.

[0xmahdirostami]

thank you @PlamenTSV , however, i think it's valid as low.

[FHieser]

As the fees set by the FeeManager is flexible in the background we cant enforce a value in the setter function.