code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

Missing maximum fee amount check #84

Closed code423n4 closed 2 years ago

code423n4 commented 3 years ago

Handle

defsec

Vulnerability details

Impact

On the documentation of the protocol, It has been written as "The protocol owner fee split must be less than 20% of the basket's license fee." However there is not limitation on the handleFee function.

Proof of Concept

  1. Navigate to "https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Basket.sol" contract.
  2. Go to the line #152.
    function changeLicenseFee(uint256 newLicenseFee) onlyPublisher public override {
        require(newLicenseFee >= factory.minLicenseFee() && newLicenseFee != licenseFee);
        if (pendingLicenseFee.licenseFee != 0) {
            require(pendingLicenseFee.licenseFee == newLicenseFee);
            require(block.number >= pendingLicenseFee.block + TIMELOCK_DURATION);
            licenseFee = pendingLicenseFee.licenseFee;

            pendingLicenseFee.licenseFee = 0;

            emit ChangedLicenseFee(licenseFee);
        } else {
            pendingLicenseFee.licenseFee = newLicenseFee;
            pendingLicenseFee.block = block.number;
        }
    }
  1. License feel should be less than %20 basket.
  2. The checks are missing on the function.

Tools Used

None

Recommended Mitigation Steps

Consider to add check according to documentation.

frank-beard commented 3 years ago

the fee limit refers to the protocol sharing fees, not the individual basket fee

GalloDaSballo commented 2 years ago

The warden is referring to a different fee, as such the finding is invalid